Skip to content

Commit

Permalink
[fei5850.1.friendlyobjectmatching] Support undefined variable compari…
Browse files Browse the repository at this point in the history
…son (#2331)

## 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: #2331
  • Loading branch information
somewhatabstract authored Oct 2, 2024
1 parent 9c68bad commit eb807af
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/old-islands-relax.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -158,6 +158,7 @@ describe("#gqlRequestMatchesMock", () => {
},
{
foo: "bar",
baz: undefined,
},
{my: "context"},
);
Expand Down
25 changes: 13 additions & 12 deletions packages/wonder-blocks-testing/src/gql/gql-request-matches-mock.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down

0 comments on commit eb807af

Please sign in to comment.