Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: adopt @cap-js/sqlite #157

Merged
merged 7 commits into from
Feb 23, 2024
Merged

test: adopt @cap-js/sqlite #157

merged 7 commits into from
Feb 23, 2024

Conversation

johannes-vogel
Copy link
Contributor

@johannes-vogel johannes-vogel commented Feb 20, 2024

REVISIT:

  • 87b2340 case sensitivity of startswith, endswith, contains
    → The functions are implemented after OData v4.01 standard which specify them as case sensitive.
  • f30e547 error for nested count in expand
    f74062f it's supported by @cap-js/sqlite

@johannes-vogel johannes-vogel requested a review from a team as a code owner February 20, 2024 14:18
@johannes-vogel johannes-vogel changed the title adopt capjs sqlite test: adopt @cap-js/sqlite Feb 23, 2024
const response = await POST('/graphql', { query })
expect(response.data).toEqual({ data, errors })
expect(response.data).toEqual({ data })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -2,6 +2,9 @@
"dependencies": {
"@cap-js/graphql": "*"
},
"devDependencies": {
"@cap-js/sqlite": "*"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these added for the plugin detection ?
If that is the reason it would be better to run the test with CDS_CONFIG to allow for testing with not just @cap-js/sqlite.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to introduce those internal flags also for open source modules.
In the meanwhile, I'd hope that there's no need anymore to switch back to the old implementation :)

{
name: 'Edgar Allen Poe',
books: {
totalCount: null, // REVISIT: expected 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the totalCount is probably being returned as [email protected]. As defined here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a BLI for us. Seems very "odata'ish" :)

@johannes-vogel johannes-vogel merged commit 9ed44e0 into main Feb 23, 2024
3 checks passed
@johannes-vogel johannes-vogel deleted the adopt-capjs-sqlite branch February 23, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants