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

refactor(datastore): Refactor test names and make subscription events more sample app like #12740

Merged
merged 12 commits into from
Jan 3, 2024

Conversation

stocaaro
Copy link
Contributor

@stocaaro stocaaro commented Dec 21, 2023

Description of changes

Outcome:

  • Test names changed to align with the harness features used per test
  • Wait for all subscriptions to be published before proceeding (remove a race condition between test assertions and subscription updates)

Description of how you validated changes

Tests updated in reference to how users receive settled updates twice in sample apps (once via mutation response and then later via subscription update).

Related changes

  1. refactor(datastore): Move conflict resolution tests to their own file #12728
  2. refactor(datastore): Refactor conflict resolution tests optimize for concise/readable #12732
  3. 🌱 refactor(datastore): Refactor test names and make subscriptions more latent / realistic
  4. test(datastore): Add OptimisticConcurrency tests #12795
  5. fix(datastore): Update outbox comparison logic to fix update versioning problems stocaaro/amplify-js#87

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@stocaaro stocaaro requested a review from a team as a code owner December 21, 2023 18:12
@stocaaro stocaaro changed the title refactor(datastore): Refactor test names and make subscriptions more latent / realistic refactor(datastore): Refactor test names and make subscription events more sample app like Dec 21, 2023
Copy link
Member

@david-mcafee david-mcafee left a comment

Choose a reason for hiding this comment

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

Since these changes result in different test assertions, I think I'd like to see a few comments explaining exactly what's happening (and why) - specific comments requesting those changes are included inline.

david-mcafee
david-mcafee previously approved these changes Jan 2, 2024
Copy link
Member

@david-mcafee david-mcafee left a comment

Choose a reason for hiding this comment

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

Thanks for responding to my previous questions on this one! Left two minor (non-blocking) suggestions.

Co-authored-by: David McAfee <mcafd@amazon.com>
@stocaaro stocaaro merged commit e295445 into aws-amplify:main Jan 3, 2024
30 checks passed
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.

None yet

3 participants