From eb807af81a06e2867c7a006422334fa3b0f83f36 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 2 Oct 2024 12:15:34 -0500 Subject: [PATCH] [fei5850.1.friendlyobjectmatching] Support undefined variable comparison (#2331) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Before this change, these two variables objects are considered different: ```ts { a: undefined, b: 1 } ``` ```ts { b: 1 } ``` This can be annoying when writing tests, because you have to explicitly set the variable to `undefined` in the expectation if the actual request ends up fetching with an explicit `undefined` value. To make GraphQL mocks easier, we now consider a missing key to be equivalent to an explicit key with an `undefined` value. This brings this approach to GraphQL mocking inline with Apollo mocks that already do this. Note that with this implementation, there's no way to assert that an explicit `undefined` variable is included in the request. We could add that if it turns out to be something we really need though I suspect that it is not. Issue: FEI-5850 ## Test plan: `yarn test` Author: somewhatabstract Reviewers: jeresig Required Reviewers: Approved By: jeresig Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Lint (ubuntu-latest, 20.x), 🚫 Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ⌛ undefined Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2331 --- .changeset/old-islands-relax.md | 5 ++++ .../gql-request-matches-mock.test.ts | 3 ++- .../src/gql/gql-request-matches-mock.ts | 25 ++++++++++--------- 3 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 .changeset/old-islands-relax.md diff --git a/.changeset/old-islands-relax.md b/.changeset/old-islands-relax.md new file mode 100644 index 000000000..d2ecd8222 --- /dev/null +++ b/.changeset/old-islands-relax.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-testing": major +--- + +When mocking GraphQL, consider explicit undefined values in a request to be equivalent to missing keys in a mock diff --git a/packages/wonder-blocks-testing/src/gql/__tests__/gql-request-matches-mock.test.ts b/packages/wonder-blocks-testing/src/gql/__tests__/gql-request-matches-mock.test.ts index a087d741c..a46cc4f7f 100644 --- a/packages/wonder-blocks-testing/src/gql/__tests__/gql-request-matches-mock.test.ts +++ b/packages/wonder-blocks-testing/src/gql/__tests__/gql-request-matches-mock.test.ts @@ -45,7 +45,7 @@ describe("#gqlRequestMatchesMock", () => { expect(result).toBe(false); }); - it.each([{foo: "bar"}, {foo: "baz", anExtra: "property"}, null])( + it.each([{foo: undefined}, {foo: "baz", anExtra: "property"}, null])( "should return false if variables don't match", (variables: any) => { // Arrange @@ -158,6 +158,7 @@ describe("#gqlRequestMatchesMock", () => { }, { foo: "bar", + baz: undefined, }, {my: "context"}, ); diff --git a/packages/wonder-blocks-testing/src/gql/gql-request-matches-mock.ts b/packages/wonder-blocks-testing/src/gql/gql-request-matches-mock.ts index 03b28f361..76ae3ae13 100644 --- a/packages/wonder-blocks-testing/src/gql/gql-request-matches-mock.ts +++ b/packages/wonder-blocks-testing/src/gql/gql-request-matches-mock.ts @@ -1,14 +1,11 @@ import type {GqlOperation, GqlContext} from "@khanacademy/wonder-blocks-data"; import type {GqlMockOperation} from "./types"; -const safeHasOwnProperty = (obj: any, prop: string): boolean => - Object.prototype.hasOwnProperty.call(obj, prop); - // TODO(somewhatabstract, FEI-4268): use a third-party library to do this and // possibly make it also support the jest `jest.objectContaining` type matching // to simplify mock declaration (note that it would need to work in regular // tests and stories/fixtures). -const areObjectsEqual = (a: any, b: any): boolean => { +const areObjectsEquivalent = (a: any, b: any): boolean => { if (a === b) { return true; } @@ -18,14 +15,18 @@ const areObjectsEqual = (a: any, b: any): boolean => { if (typeof a !== "object" || typeof b !== "object") { return false; } + + // Now, we need to compare the values of the objects. + // We can't just compare key sets as we want to consider an explicit + // key with an undefined value to be the same as a missing key. + // It makes for a nicer API when defining mocks. const aKeys = Object.keys(a); const bKeys = Object.keys(b); - if (aKeys.length !== bKeys.length) { - return false; - } - for (let i = 0; i < aKeys.length; i++) { - const key = aKeys[i]; - if (!safeHasOwnProperty(b, key) || !areObjectsEqual(a[key], b[key])) { + + const allKeys = new Set([...aKeys, ...bKeys]); + + for (const key of allKeys) { + if (!areObjectsEquivalent(a[key], b[key])) { return false; } } @@ -52,7 +53,7 @@ export const gqlRequestMatchesMock = ( // we just assume it matches everything. if (mock.variables != null) { // Variables have to match. - if (!areObjectsEqual(mock.variables, variables)) { + if (!areObjectsEquivalent(mock.variables, variables)) { return false; } } @@ -61,7 +62,7 @@ export const gqlRequestMatchesMock = ( // we just assume it matches everything. if (mock.context != null) { // Context has to match. - if (!areObjectsEqual(mock.context, context)) { + if (!areObjectsEquivalent(mock.context, context)) { return false; } }