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

feat(testing): Implement "assertSnapshot" #2039

Merged
merged 47 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
73467da
wip: Implement assertSnapshot
hyp3rflow Mar 17, 2022
4d8bf6c
wip: Get full ident from recursive getName function
hyp3rflow Mar 17, 2022
4833857
format
hyp3rflow Mar 18, 2022
31abdb8
Apply bcheidemann's review
hyp3rflow Mar 18, 2022
418cc43
proposal: Use `Deno.args` for update and `globalThis.onunload` instea…
hyp3rflow Mar 22, 2022
58df39e
Add count
hyp3rflow Apr 6, 2022
f191dd3
jest snapshot style
hyp3rflow Apr 6, 2022
9acd82c
add test case
hyp3rflow Apr 6, 2022
ea2479f
refactor: apply review comments 1
bcheidemann Apr 15, 2022
2b717b6
refactor: remove duplicate format calls
bcheidemann Apr 15, 2022
46b0b9a
feat: show number of snapshots updated
bcheidemann Apr 15, 2022
a199814
fix: only call writeSnapshotFileSync once per test
bcheidemann Apr 15, 2022
d36765c
refactor: format files
bcheidemann Apr 15, 2022
9df0184
fix: output snapshots to __snapshots__ directory
bcheidemann Apr 15, 2022
2538887
fix: handle missing snapshot file
bcheidemann Apr 15, 2022
c710140
refactor: group variables into assert snapshot context
bcheidemann Apr 15, 2022
c53bb73
refactor: use maps
bcheidemann Apr 15, 2022
140bd96
refactor: tidyup
bcheidemann Apr 15, 2022
b6d55e3
docs: add documentation for assertSnapshot
bcheidemann Apr 15, 2022
d024982
refactor: move code to be adjacent to other asserts
bcheidemann Apr 15, 2022
da8e582
fix: single line snapshots captured correctly
bcheidemann Apr 15, 2022
03b250e
fix: escape characters correctly
bcheidemann Apr 15, 2022
f4b4174
fix: type fix for assertSnapshot example
bcheidemann Apr 15, 2022
063261b
Merge branch 'main' of github.com:denoland/deno_std into feat/assertS…
bcheidemann Apr 15, 2022
a4bf357
Merge branch 'feat/assertSnapshot' of github.com:hyp3rflow/deno_std i…
bcheidemann Apr 15, 2022
6ca3bd1
refactor: format files
bcheidemann Apr 15, 2022
5ec8e41
fix: snapshot path to match Jest
bcheidemann Apr 15, 2022
2f0cc55
fix: improve assertSnapshot output
bcheidemann Apr 15, 2022
9e4390c
docs: add better documentation for assertSnapshot
bcheidemann Apr 17, 2022
fe87cfa
Merge branch 'main' of github.com:denoland/deno_std into feat/assertS…
bcheidemann Apr 17, 2022
b1782f8
docs: add link to Snapshot Testing section
bcheidemann Apr 17, 2022
21b96d0
test: add test data to map
bcheidemann Apr 17, 2022
7ad5a8b
refactor: move assertSnapshot to snapshot.ts
bcheidemann Apr 17, 2022
666f32d
fix: snapshot import on windows
bcheidemann Apr 17, 2022
3902ec7
docs: use "t" instead of "test" for assertSnapshot example
bcheidemann Apr 19, 2022
e13af8a
docs: use "t" instead of "test" for assertSnapshot example
bcheidemann Apr 19, 2022
158b8d5
docs: fix typescript issues in docs
bcheidemann Apr 19, 2022
c3264dd
fix: write snapshot file to correct path
bcheidemann Apr 19, 2022
e1155a0
fix: add training new line to snapshot files
bcheidemann Apr 19, 2022
fddf34d
refactor: use addEventListener for snapshot teardown
bcheidemann Apr 19, 2022
24f1868
test: add tests for assertSnapshot update flag
bcheidemann Apr 19, 2022
9c34b02
refactor: fromat files
bcheidemann Apr 19, 2022
03d2a4c
docs: fix snapshot test examples
bcheidemann Apr 19, 2022
0a66471
docs: fix assertSnapshot example and
bcheidemann Apr 19, 2022
4542ea9
test(node): update global var leak detection
kt3k Apr 20, 2022
faeb691
fix: fix snapshot assertions in --update testing
kt3k Apr 20, 2022
26aba27
fix: mask more timing notations
kt3k Apr 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions testing/asserts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
stripColor,
white,
} from "../fmt/colors.ts";
import { fromFileUrl, parse, join } from "../path/mod.ts";
import { ensureFile, ensureFileSync } from "../fs/mod.ts";
import { diff, DiffResult, diffstr, DiffType } from "./_diff.ts";

const CAN_NOT_DISPLAY = "[Cannot display]";
Expand Down Expand Up @@ -864,3 +866,59 @@ export function unimplemented(msg?: string): never {
export function unreachable(): never {
throw new AssertionError("unreachable");
}

let snapshotFile: Record<string, unknown> | undefined = undefined;
const updatedSnapshotFile: Record<string, unknown> = {};
// const snapshotMap: Record<string, number> = {};

export async function assertSnapshot(context: Deno.TestContext, actual: unknown) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we also expose a function called assertSnapshotSync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good! I think we can implement that after denoland/deno#14007 merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's hard to do this with the way we now read the snapshot file. I think this can probably be implemented separately if it's needed

const name = getName(context);
// const count = getCount(name);

if (!snapshotFile) {
const snapshotPath = getSnapshotPath();
await ensureFile(snapshotPath);
const file = await Deno.readTextFile(snapshotPath);
snapshotFile = file ? JSON.parse(file) : {};
}
const snapshot = snapshotFile?.[name];
if (!Deno.args.includes('--update')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!Deno.args.includes('--update')) {
if (!Deno.args.some(arg => arg === '--update' || arg === '-u')) {

Could do something like this to support -u and --update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will handle this after denoland/deno#14007 merged!

Copy link
Member

Choose a reason for hiding this comment

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

BTW to pass -u option to Deno.args, you need to invoke test command like deno test -- -u because the flags before -- are consumed by Deno CLI itself. I'm ok with that for the first pass implementation, but it might be non ideal solution, I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I also noticed that the flag for updating snapshot can affect other codes that wanted to test.. but I think that using Deno.args is better than making an option on Deno test cli.

Copy link
Contributor

Choose a reason for hiding this comment

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

Imo this is the best solution for first pass. But if/when snapshot testing is a more established then we should look to implement this on the Deno CLI instead; as @hyp3rflow mentioned, this can affect test execution.

assertEquals(actual, snapshot);
Copy link
Contributor

Choose a reason for hiding this comment

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

The current implementation will error on:

const actual = {
  fn: () => { /* ... */ }
}

As snapshots need to be represented as strings anyway, I would argue it makes more sense to use the _format helper here to convert the snapshot into a string. We could store the snapshot like this:

{
  "Snapshot Test > babo > merong": [
    "{",
    "  b: 2,",
    "  c: 3,",
    "}"
  ]
}

This would enable us to catch a much wider range of errors in our snapshots. E.g. the following would error on assertSnapshot:

// First run
const actual = {
  testClass: class Test {}
}
// --snip--
assertSnapshot(test, actual)

// Second run
const actual = {
  testClass: class NotTest {}
}
// --snip--
assertSnapshot(test, actual)

But would not error (or worse, always error) if we only store the JSON representation.

Additionally, this representation enables us to use the diff and buildMessage to produce helpful output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Deno.inspect() can be a good choice for serialization method. What do you think? @bcheidemann

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this would definitely do the job. I would suggest to use the _format helper which wraps Deno.inspect. This ensures the output will be consistent with other asserts 🙂

} else {
let isEqual = true;
try {
assertEquals(actual, snapshot);
} catch {
isEqual = false;
}
if (!isEqual) console.info("Snapshot updated", name);
}
updatedSnapshotFile[name] = actual;
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean this implementation supports only 1 snapshot per test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the test case mean one Deno.test?
This implementation supports 1 snapshot per test module (one .test.ts file)

for example, if I test a.test.ts with --update arg,

Deno.test("Snapshot Test A", async (t) => {
  await assertSnapshot(t, { a: 1, b: 2 });
  await t.step("yaho", async (t) => {
    await assertSnapshot(t, { b: 2, c: 3 });
    await t.step("haha", async (t) => {
      await assertSnapshot(t, { b: 2, c: 4 });
    });
  });
});

Deno.test("Snapshot Test A2", async (t) => {
  await assertSnapshot(t, { a: 1, b: 2 });
  await t.step("what", async (t) => {
    await assertSnapshot(t, { b: 2, c: 3 });
    await t.step("good", async (t) => {
      await assertSnapshot(t, { b: 2, c: 4 });
    });
  });
});

the output, a.test.snap will be like this

{
  "Snapshot Test A": {
    "a": 1,
    "b": 2
  },
  "Snapshot Test A > yaho": {
    "b": 2,
    "c": 3
  },
  "Snapshot Test A > yaho > haha": {
    "b": 2,
    "c": 4
  },
  "Snapshot Test A2": {
    "a": 1,
    "b": 2
  },
  "Snapshot Test A2 > what": {
    "b": 2,
    "c": 3
  },
  "Snapshot Test A2 > what > good": {
    "b": 2,
    "c": 4
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

How about the below case? I think the second assertSnapshot call overwrites the snapshot from the first call

Deno.test("Snapshot Test A", async (t) => {
  await assertSnapshot(t, { a: 1, b: 2 });
  await assertSnapshot(t, { c: 3, d: 4 });
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha! Sorry, now I know what you mean. That case will be handled with commented count. (Currently not implemented perfectly so I commented) @kt3k

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok. I see. getCount does that 👍


globalThis.onunload = writeSnapshotFileSync;

// function getCount() {
// const count = snapshotMap?.[name] ? snapshotMap[name] : 1;
// snapshotMap[name] = count + 1;
// return count;
// }

function getName(context: Deno.TestContext): string {
if (context.parent) return `${getName(context.parent)} > ${context.name}`;
return context.name;
}

function getSnapshotPath() {
const testFile = fromFileUrl(context.origin);
const parts = parse(testFile);
return `${join(parts.dir, parts.name)}.snap`;
}

function writeSnapshotFileSync() {
const snapshotPath = getSnapshotPath();
ensureFileSync(snapshotPath);
const file = JSON.stringify(updatedSnapshotFile, null, 2);
Deno.writeTextFileSync(snapshotPath, file);
console.log('Snapshot updated!');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log('Snapshot updated!');
console.log(green(bold(` > ${n} snapshots updated.`)));

Maybe we could mimic Jest here and log how many snapshots were updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

}
}
14 changes: 14 additions & 0 deletions testing/asserts_test.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"Snapshot Test": {
"a": 1,
"b": 2
},
"Snapshot Test > babo": {
"b": 2,
"c": 3
},
"Snapshot Test > babo > merong": {
"b": 2,
"c": 4
}
}
11 changes: 11 additions & 0 deletions testing/asserts_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
assertNotStrictEquals,
assertObjectMatch,
assertRejects,
assertSnapshot,
assertStrictEquals,
assertStringIncludes,
assertThrows,
Expand Down Expand Up @@ -1408,3 +1409,13 @@ Deno.test("Assert Is Error with custom Error", () => {
'Expected error to be instance of "CustomError", but was "AnotherCustomError".',
);
});

Deno.test("Snapshot Test", async (t) => {
await assertSnapshot(t, { a: 1, b: 2 });
await t.step("babo", async (t) => {
await assertSnapshot(t, { b: 2, c: 3 });
await t.step("merong", async (t) => {
await assertSnapshot(t, { b: 2, c: 4 });
});
});
});