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(datastore): Add OptimisticConcurrency tests #88

Closed

Conversation

stocaaro
Copy link
Owner

@stocaaro stocaaro commented Dec 22, 2023

Warning

PR reconstructed against main - Please see aws-amplify#12795

Description of changes

Outcomes:

  • Update graphql service fake to mimic OptimisticConcurrency(OCC) conflict resolution
  • Add OptimisticConcurrency (OCC) tests for all cases covered by the existing Automerge tests

This change adds an outer block in the tests, which change whitespace for the entire file. I strongly recommend reviewing without whitespace changes.
image

Description of how you validated changes

By comparing sample app results to test expectations and iterating until they align.

There is existing odd behavior where errors occur at in low latency conditions where the retry message includes null values for fields, which overwrites external updates. I will do sample app testing to confirm that this is consistent with the real world behavior.

Related changes

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

Checklist

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

@@ -730,16 +785,28 @@ export class FakeGraphQLService {
}
}

this.notifySubscribers(tableName, type, data, selection, ignoreLatency);
this.notifySubscribers(
Copy link
Owner Author

Choose a reason for hiding this comment

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

This isn't right yet. Isn't the sub only updated when there is data? In the reject case, I don't think an update gets posted. Need to check with the sample app.

stocaaro and others added 10 commits January 3, 2024 10:55
… more sample app like (aws-amplify#12740)

* Improve comments and get rid of unused features

* Updates from comments

* Update packages/datastore/__tests__/helpers/UpdateSequenceHarness.ts

Co-authored-by: David McAfee <mcafd@amazon.com>

* Merge fix

* test(datastore): Refactor test names and make subscriptions more latent / realistic

* Updates from comments

* add clarifying comment

* Add a comment explaining input delay to the first reference

* Update packages/datastore/__tests__/helpers/UpdateSequenceHarness.ts

Co-authored-by: David McAfee <mcafd@amazon.com>

---------

Co-authored-by: David McAfee <mcafd@amazon.com>
Co-authored-by: David McAfee <mcafd@amazon.com>
@stocaaro stocaaro force-pushed the 04-consecutive-updates-take2 branch from 7cb40e3 to 23b2ac0 Compare January 4, 2024 17:13
@stocaaro stocaaro closed this Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant