[graphql-fixtures] Graphql fixtures/seed collision fix

[Flufd]

Description

Fixes #2311

Uses a quick hash of the strings that make up a graphQL object's keypath to generate a random seed for that object, instead of a sum of all of the characters values.

I think this is a breaking change, only if people are asserting on the random values that are already generated.

TODO: changelog, once I know if this is considered breaking.

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
• Minor: New feature (non-breaking change which adds functionality)
• Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

[BPScott]

Looking good! I've just added a commit to remove the need for an as any cast in the test.

I think this is a minor change. End users shouldn't be relying on the randomly returned filler data. If there's some data that they want to fix then they should specify it.

You can cut a beta release and test this in web by commenting /snapit on the PR.

Add a changeset (yarn add changeset and follow instructions). and I think we're good to go. See https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md for more info.

[BPScott]

/snapit

[github-actions]

🫰✨ Thanks @BPScott! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

[BPScott]

Give that version of graphql-fixtures a go in web and check if the changing of the seed generation has a painful effect on tests.

[Flufd]

thanks @BPScott. Doesn't look like graphql-fixtures was cut in that release. I had a quick look why that might be, but couldn't figure it out.

[BPScott]

ah, duh, I forgot that we need a changeset in this branch that mentions graphql-fixtures before snapit shall pick up that the package needs to be released

(new toy, still internalising how it works)

[BPScott]

/snapit

[github-actions]

🫰✨ Thanks @BPScott! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add [email protected]

yarn add [email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add [email protected]

yarn add [email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

[BPScott]

yarn add [email protected]

there we go

[BPScott]

Good news: I think the code change is good. Improving the seed generation so it results in fewer trivial collisions (e.g. a[0].b[1] collides with a[1].b[0] and a[12] collides with a[21]) is a good thing.

Bad news: I suspected that consumers would rely on this buggy behaviour in some places. However it is worse than I thought - introducing this change in web results in 370ish test failures. I am pretty confident that this is web doing the wrong thing here, and that it is web's tests that need to be updated. However the sheer number of tests suggests that relying on this buggy behaviour is easier than I anticipated, and thus a release of graphql-fixtures containing change will require more work to merge cleanly. With that in mind, I think the least surprising thing is to claim this is a major version bump and write something more detailed in the changeset to describe why this would cause test failures.

Worse news: We are working on apollo3 in quilt and web and I don't want the release of this change to block the testing of that work. Thus I do not want to release this change until a version of graphql-fixtures containing the change can be cleanly used in web without test failures. That means you will need to go fix all the tests in web to not rely on this buggy behaviour, before we merge this PR.

Proposed Plan

• The CI for https://github.com/Shopify/web/pull/67518 shows all the test failures in web as a result of this update. Apply fixes for these tests, and backport them to the main branch (as it is possible to write fixed tests that work before and after this change).
• Once we get to a point where a PR containing just a graphql-testing bump passes CI, then we we can merge and release this change as a major version.
• Update web to use that new major version.

Are you able to corral a team to make these fixes in web?

[BPScott]

Marking this as blocked, till we update web to not rely on this buggy behaviour that we're removing.

We'll also want to update this to be a major change in the changeset

[shopify-shipitnext]

🔎 View this PR in Shipit Next.

ℹ️ Expand to learn how to deploy and handle emergencies using Shipit Next

Overview

Shipit Next will merge your code on your behalf because this repository uses Shipit Next and its merge queue.

To ship this PR, you can either:

• Comment /shipit to enqueue this PR for merging.
• Go to this PR in Shipit Next and click on the "Ship It" button in the top-right corner.

Comment Commands

• /shipit: Enqueue this PR into the merge queue where it will eventually be merged and deployed.
• /cancel: Eject this PR from the merge queue and rebuild PRs that were enqueued after this PR.
• /shipit --jump-queue: Enqueue this PR at the top of the merge queue where it will be included in the next deploy. Use this for non-emergency situations.
  - Emergency handling procedure for this command can be found here.
• /shipit --emergency: Merge this PR directly into main and deploy to all environments once all require_for_emergency CI checks pass. Please be aware that changes deployed with this command will not be automatically rolled back.
  • Emergency handling procedure for this command can be found here.

Commands exclusive to Deploy Before Merge

• /cancel --emergency: Eject this PR from the merge and rollback any deployments containing this PR.

---

Documentation

• ℹ️ You can read how the merge queue works in Vault here.
• ⚠️ Emergency handling documentation in the Vault describes how to handle emergencies, e.g. failed canaries, broken code in production, etc.

Questions or feedback?

• Non-urgent questions can be submitted in our Discourse.
• Feedback can be submitted in our frustrations Google form.
• If you have feature requests, you may add them or vote for them in our GitHub repository.