-
Notifications
You must be signed in to change notification settings - Fork 4
chore: Add support for persistent store contract tests. #502
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
Merged
+333
−3
Merged
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
2edfc8d
chore: Add support for persistent store contract tests.
kinyoklion baeb833
Merge main into rlamb/persistence-contract-tests
devin-ai-integration[bot] eb918dd
fix: add use_redis flag to server-redis.yml workflow steps
devin-ai-integration[bot] d960e6f
fix: update OpenSSL chocolatey version from 3.5.4 to 3.6.1 for Window…
devin-ai-integration[bot] 14b2e22
Revert "fix: update OpenSSL chocolatey version from 3.5.4 to 3.6.1 fo…
devin-ai-integration[bot] d09ca12
Merge remote-tracking branch 'origin/main' into rlamb/persistence-con…
devin-ai-integration[bot] b9a13e4
fix: move enable_persistence_tests to Redis contract test jobs
devin-ai-integration[bot] 9a95f3e
fix: change use_redis default to empty string to preserve auto-detection
devin-ai-integration[bot] bdcb385
fix: update contract-tests action to v1.3.0 for enable_persistence_te…
devin-ai-integration[bot] 8d4e39c
fix: correct TTL field name mismatch in persistent cache config
devin-ai-integration[bot] e5bf095
fix: allow lazy load evaluations when $inited key is not set
devin-ai-integration[bot] cc44864
fix: move warn-and-proceed logic to evaluation path for lazy load
devin-ai-integration[bot] 6cf1a3c
Merge branch 'devin/1773359647-lazy-load-initialized-warning' into rl…
devin-ai-integration[bot] 3fe6eb3
chore: add suppressions for read-write persistent store tests
devin-ai-integration[bot] fc75921
Merge origin/main into rlamb/persistence-contract-tests
devin-ai-integration[bot] d308c56
chore: add DynamoDB persistent store contract test support
devin-ai-integration[bot] 620d32b
ci: rename dynamodb contract test job
devin-ai-integration[bot] 8b4f028
chore: Run persistence contract tests against both harness versions
devin-ai-integration[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2tests, and one forv3, which includes fdv2. But I think these new workflows only cover thev2tests without fdv2. If we add redis/dynamodb as a second dimension, should we set up a test matrix with all the combinations, or...?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both good catches — addressed in 8b4f028.
It was partly redundant: the store jobs also re-ran the whole non-persistence v2 suite that
libs/server-sdkalready runs. They now pass-run=persistent.data.store, so each job only runs the persistence suite (the store-specific dimension). I also dropped the rediscontract-tests-curljob — the HTTP backend has no bearing on store behavior andlibs/server-sdkalready covers curl.Each store workflow is now a matrix over harness
v2/v3, which required a code fix: the v3 harness nests the store config underdataSystem.store.persistentDataStore(plusstoreMode), so previously every v3 persistence test failed. The service now reads either location. Current per-store results:The v3 suppressions (
persistence-suppressions-fdv2.txt) are the read-write tests plus allwith data sourcecases — the data system method is a variant (Lazy Load or Background Sync or FDv2), so a store can't be combined with a data source, and the store integrations are read-only sources. Both are SDK gaps rather than harness/test-service gaps.One caveat with
-run: if the harness ever renames thepersistent data storegroup, these jobs would pass while running zero tests.