refactor(datastore): Refactor conflict resolution tests optimize for concise/readable

[stocaaro]

Description of changes

Outcome:

• ~1300 lines -> ~700 lines aimed at maintaining readability. All assertions are the same as they where before this refactor.
• All test assertions are maintained with the same expected result for each test

We want to add OCC tests in a subsequent change. Rather than duplicating these tests as they where, I have refactored them, extracting out shared logic that isn't core to the testing story into a test harness aimed to enable/clarify these tests specifically.

Description of how you validated changes

The test suite continues to run to success

Related changes

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

Checklist

• PR description included
• yarn test passes
• Tests are changed or added

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

[svidgen]

I have a couple opinions, but non-blocking. Overall I think this is trending in the right direction, and don't want to impede the positive changes.

[david-mcafee]

At a high-level, I would argue that some of these changes make the tests less readable than before, but perhaps that’s just me!!! Understanding what’s happening in DataStore at any given moment is quite difficult with all of the async concurrent behavior, and though abstracting the repeated logic out of the tests means less lines of code, I would argue that I (personally) have a harder time understanding what’s happening at each step, and I worry that we'll make updates to the tests in the future that will result in different behavior being tested (and thus would no longer identify a regression). If I was paged at 3am and had to figure out what’s broken with DataStore, I’d want the tests to be as easy to reason about as possible, with no additional cognitive overload, even if that means tests are repeating themselves too much.

At a minimum, I think we should 1) keep the manual pauses between DataStore updates (as an example, some of these tests now pause when they should not), 2) re-add some of the comments back into the tests (see specific feedback below, but adding comments to the helpers does not work because we do not always wait on the outbox, etc), and 3) go back to setting the latencies after the initial save has completed.

There are a few places where we are potentially no longer testing the same behavior as before. Even though the assertions are the same, that doesn’t necessarily mean that DataStore is processing the updates in the same way - for instance, the assertions across most of the tests are the same, even though the tests are testing different DataStore patterns. I also think we should continue to supply the "external" update number to the assertion testing that the fake service has settled so that, in the future, we can update the fake service to differentiate between primary client / external client updates, since that's a weak spot in these tests (and I would argue one of the most important updates we should make before committing the fix).

Lastly, since this is a pretty big refactor, I think it would be smart to test these "manually" by adding a very long pause at the end of each of these tests to make sure no other subscription messages eventually come through.

[david-mcafee]

LGTM!! 🎉