snapshot testing

[brandonkal]

A nice to have feature would be Jest-like snapshot testing in the std/testing library.

[ry]

I don't know what that means... can you explain more?

[bartlomieju]

Ref https://github.com/denoland/deno_std/issues/346

[brandonkal]

@ry Basically when you call expect(x).toMatchSnapshot() it serializes whatever x is and stores it in a state file. Here's an example.

The premise is that you take a snapshot of the state when things are how they should be, and if they later differ, the test fails. You also have the option to update the snapshot if you desire the change or to define a custom serializer.

[brandonkal]

Also the option to run a specific test by name (filter) would be nice. Failfast marks all other tests as failed which is confusing. I'm finding myself commenting tests out to iterate on broken ones.

[KSXGitHub]

What do you think the API should look like?

I propose this:

import { matchSnapshot } from 'https://deno.land/std/testing/snapshot.ts'
matchSnapshot('filename.snapshot', myObject)

deno test # would fail if snapshot is outdated
deno test -u # would update snapshot if it is outdated

[brandonkal]

I would propose the API be closer to Jest's .toMatchSnapshot(). So there is no need to specify where to store the snapshot.

A similar structure would be implied

my_test.ts
__snapshots__
|- my_test.ts.denosnap

[KSXGitHub]

How would a std lib knows where the test file or snapshot file is located? The only way to know is for the test file to provide import.meta.url, which is just snapshot location with extra steps.

[brandonkal]

import.meta.url gives you the filepath of the test file. From there you easily convert it to containing folder + __snapshots__/filename.denosnap

You could also do something like:

import { makeSnapshotter } from 'https://deno.land/std/testing/snapshot.ts'
const snap = makeSnapshotter('./__snapshots__')
snap(myObject)

[KSXGitHub]

@brandonkal That's what I said: "snapshot location with extra steps".

import.meta.url gives you the filepath of the test file

import.meta.url only gives filepath of the test file if it was the test file that invoke it. Within an /std lib, it will gives you a URL starts with https://deno.land/std. The library can only get import.meta.url of the test file if the test file give it, which is equivalent to giving location of the snapshot file.

---

Both my design (the test file give location of the snapshot file) and yours (the test file give import.meta.url) have a limitation: The test file may decide to give a URL that does not match its location, making detection of redundant snapshot files impossible.

I think we better come up with a better design which may allow testing lib to determine snapshot file location without help from the test file, but it would require change in Deno core:

• deno test runs every test file in isolation from each other, with separate globalThis.
• Every isolate has a way to tell testing lib location of the test file, be it window.location, Deno.scriptLocation(), Deno.env().DENO_TEST_LOCATION, or what have you.

[nayeemrmn]

Just create explicit paths before you pass to std.

matchSnapshot(new URL(import.meta.url, "testdata/filename.denosnap").pathname, myObject);
// or
matchSnapshot("__snapshots__/filename.denosnap", myObject); // from cwd

Implicitly adding a directory (other than cwd) offers little control and resolving from main module seems like bad practice.

[KSXGitHub]

@nayeemrmn What you proposed was my idea. However, as I also wrote in https://github.com/denoland/deno/issues/3635#issuecomment-599023737, it has limitation: Deletion of redundant snapshot file is impossible (whereas in Jest, detecting outdated snapshot file is easy, because test file cannot control where to write snapshot to).

[brandonkal]

The point is you are not supposed to have control on where the snapshot is stored. It's an implementation detail. All files have a unique import.meta.url. From there all you need is a function which maps a import.meta.url to its snapshot storage folder. It is up to the testing library where to store the snapshot. Typically it is right next to the test file so it can be committed in git.

matchSnapshot("__snapshots__/filename.denosnap", myObject)

That is verbose and gets messy quickly.

[KSXGitHub]

@brandonkal Are we just repeating ourselves now? Yes, all import.meta.url are unique, but test file can choose to pass an arbitrary string instead of import.meta.url. How could a testing lib know if a string is truly import.meta.url or not?

If you so value the lack of control of snapshot location (just like I do), then I beg you to focus the discussion on https://github.com/denoland/deno/issues/3635#issuecomment-599023737 (the second part)

[brandonkal]

I see what you are saying now. That would be a good way to solve the issue.

Just spitballing here but you could also solve this by having each test file could look like this:

import { makeSnapshotter } from 'https://deno.land/std/testing/snapshot.ts'

export default function run(name) {
  const snap = makeSnapshotter(name)
  /// The tests
  snap(myObject)
}

if (import.meta.main) run(import.meta.main)

Then the .deno.test.ts file that is generated changes from:

import "file:///the_test.ts"

to

import test1 from "file:///the_test.ts"
test1("file:///the_test.ts")

It's a bit more explicit but you are not injecting magic globals into the test environment. Though I wouldn't mind that either.

[nayeemrmn]

It's an implementation detail.

But we go as far as to commit them to git, I kind of don't buy it as an implementation detail nor does automatic deletion sound so important... anyway I didn't know anything about snapshot testing before yesterday so I'll back out :p but the more I've learnt about them the more they seem like a crap jest thing. Ignore me.

[KSXGitHub]

@nayeemrmn

You seem to dislike snapshot. Yet even Deno itself uses something similar to snapshots (cli/tests/*.out).

[KSXGitHub]

Anyway, snapshot testing may or may not be in the standard library, either way is fine to me. But I think isolating every test from each other is a good idea in general, wouldn't you agree? With tests being isolated, it would then be possible for others to create a snapshot testing library that does not ask test file for import.meta.url.

[nayeemrmn]

similar to snapshots (cli/tests/*.out).

Hardly!

[kitsonk]

Yeah, that is like saying any assertion against a string is like snapshotting. I don't think it is a good comparison at all.

I haven't voiced an opinion yet, because I am very well aware of snapshotting, but have never seen it deliver value. Developers simply rebaseline when something unexpected happens. That are, in my opinion, a false security blanket.

It certainly feels like something that should start in the wider community versus being in std in my opionion.

[bartlomieju]

It certainly feels like something that should start in the wider community versus being in std in my opionion.

I completely agree with @kitsonk on this one; it should start as third-party lib instead of std/ lib. Only then if it's very desirable we can think about introducing it into std/.

I'm gonna close this issue now, but feel free to revisit if such library surfaces!

[bcheidemann]

@bartlomieju some libraries have now been created (e.g. klick) so maybe it's time to re-visit this discussion. I think it's an important feature for use cases involving SSR and SSG so Deno should consider implementing it as part of the standard library.

[bartlomieju]

@bartlomieju some libraries have now been created (e.g. klick) so maybe it's time to re-visit this discussion. I think it's an important feature for use cases involving SSR and SSG so Deno should consider implementing it as part of the standard library.

Yes, I've come around on this topic, I would welcome a PR to deno_std implementing this, seems useful for testing CLI programs too

[bcheidemann]

@bartlomieju some libraries have now been created (e.g. klick) so maybe it's time to re-visit this discussion. I think it's an important feature for use cases involving SSR and SSG so Deno should consider implementing it as part of the standard library.

Yes, I've come around on this topic, I would welcome a PR to deno_std implementing this, seems useful for testing CLI programs too

That's great! 😁

---

As I understand it, there are currently two philosophies:

The "Jest" way We should follow the lead of Jest and create the snapshot file "next to" the test file. For example, if the test exists at project/__test__/test.ts then the snapshot files could be created in the folder project/__test__/__snapshots__/test.snap. The test should not be able to (or at least should not need to) control where the snapshots are created. This would ideally look something like this:

assertSnapshot(obj);

Passing import.meta.url to assertSnapshot This has largely been proposed as a way to sidestep the technical difficulties of implementing the above. A syntax for assertSnapshot has been proposed which is something like:

assertSnapshot(import.meta.url, obj);

---

Currently, klicks implementation more closely resembles the "Jest" way. However, to achieve this, it wraps the Deno.test function. In my opinion, this is undesirable and it should be possible to use the assertSnapshot function in an undercoated Deno.test context.

Having read through the source for klick, I can't see any obvious reason why wrapping Deno.test is necessary. Though, I haven't actually tried to implement a solution without a Deno.test wrapper so I may be mistaken here.

My main issue with klicks implementation is that it effectively gets determines the value of import.meta.url by throwing and then catching an error and reading the stack trace.

This is fine for a third party library but I'm skeptical of implementing this approach in the standard library. For a start, this is prone to breaking if the format of the stack trace changes in future.

---

I'm very new to Deno so I know almost nothing about how the test runner works internally. It would be really useful if anyone could point me in the direction of some useful resources for better understanding the internals of the test runner - though I'm fully aware that likely no such resources exist and I just need to read the code! 😋

[bcheidemann]

I see what you are saying now. That would be a good way to solve the issue.

Just spitballing here but you could also solve this by having each test file could look like this:

import { makeSnapshotter } from 'https://deno.land/std/testing/snapshot.ts'

export default function run(name) {
  const snap = makeSnapshotter(name)
  /// The tests
  snap(myObject)
}

if (import.meta.main) run(import.meta.main)

Then the .deno.test.ts file that is generated changes from:

import "file:///the_test.ts"

to

import test1 from "file:///the_test.ts"
test1("file:///the_test.ts")

It's a bit more explicit but you are not injecting magic globals into the test environment. Though I wouldn't mind that either.

Had a quick skim through the Deno source and it looks like this file no longer exists. Seems like Deno now runs tests isolated from one another.

---

In the spirit of spitballing, I've considered to following options for "magic globals".

Option 1 - Deno global

The Deno global seems to be shared by all imports of the test script so we could set a property on this to be the value of import.meta.url for the test script - for arguments sake assume Deno.testMeta.url = import.meta.url. To avoid confusion, this would not be implemented in the standard library or the test script but in a similar way to how Deno.version.deno is set.

This doesn't work because people might want to run predefined subsets of tests by importing all the test files into a parent test script like this:

// <rootDir>/__test__/a.subset.test.ts
import "./a.test.ts";
import "./b.test.ts";

// <rootDir>/b.subset.test.ts
import "./__test__/a.test.ts";
import "./__test__/c.test.ts";

This is problematic because we will then end up with duplicate snapshots:

// <rootDir>/__test__/__snapshots__/a.subset.test.ts.snap

// snapshots for a.test.ts
// snapshots for b.test.ts

// <rootDir>/__snapshots/b.subset.test.ts.snap`

// snapshots for a.test.ts
// snapshots for c.test.ts

Furthermore if we then run just a.test.ts we will end up with another set of snapshots for a.test.ts.

In my opinion, test snapshots should always be written to and read from the same file system path, regardless of how the test is run.

Option 2 - importee.meta.url

For arguments sake, lets assume we implement some way for a given script to access the URL of the file that imported it.

This still wouldn't work because a common practice is to import all dependencies once from a dependencies file and re-export them. For example:

// <rootDir>/deps.ts
export { assertSnapshot } from "https://deno.land/std@0.126.0/testing/assertSnapshot.ts";

// <rootDir>/__test__/test.ts
import { assertSnapshot } from "../deps.ts";

In this case, the snapshot would incorrectly be created in <rootDir>/__snapshots__/deps.ts.snap instead of in <rootDir>/__test__/__snapshots__/test.ts.snap.

Option 3 - Deno.test to set a "magic global"

Maybe there's some way the Deno.test function could set a "magic global" capturing the current import.meta.url. I'm assuming this would be technically possible but I'm not sure how practical it would be as I'm currently unsure where Deno.test is implemented. This global could then be consumed by assertSnapshot in order to determine where to output the test snapshots to.

---

The above are just my initial thoughts based on very little research and knowledge of Deno. Pleas correct me if I've gotten anything wrong! 😋

[bartlomieju]

@bcheidemann explicitly passing import.meta.url would be the way to go, we avoid adding "magic" variables and messing with the test environment at all cost - so I'm not in favor of any of the three options you proposed. Being explicit is better in this situation as it wouldn't lead to some "gotchas"

[bcheidemann]

@bcheidemann explicitly passing import.meta.url would be the way to go, we avoid adding "magic" variables and messing with the test environment at all cost - so I'm not in favor of any of the three options you proposed. Being explicit is better in this situation as it wouldn't lead to some "gotchas"

@bartlomieju I agree, this is the best solution and also the easiest to implement.

So syntax would be:

// test.ts
assertSnapshot(import.meta.url, obj);

// Validate snapshots (default behavior)
deno test --allow-read

// Update snapshots
deno test --allow-read --allow-write -- -u
deno test --allow-read --allow-write -- --update

klick also has a "refresh" option but I think this can be added later if desirable.

I would love to raise a PR if you're happy with the above?

[bartlomieju]

@bcheidemann feel free to open a PR in deno_std and let's work from there.

[bcheidemann]

@bartlomieju usual format for snapshots is to include the test name in the snapshot file. Seems like this is why klick implements a wrapper function. As I see it, our options are:

• Implement a wrapper function
• Pass the test name as well as import.meta.url
• Change the implementation of Deno.test to somehow make the test name accessible within the text context (maybe through this.test.name? but that wouldn't work for arrow functions e.g. Deno.test(() => ...))

Thoughts?

[hyp3rflow]

How about using Deno.mainModule instead of import.meta.url? Is there any reason not to use it?

[bartlomieju]

@bartlomieju usual format for snapshots is to include the test name in the snapshot file. Seems like this is why klick implements a wrapper function. As I see it, our options are:

• Implement a wrapper function
• Pass the test name as well as import.meta.url
• Change the implementation of Deno.test to somehow make the test name accessible within the text context (maybe through this.test.name? but that wouldn't work for arrow functions e.g. Deno.test(() => ...))

Thoughts?

Sorry I missed your reply.

Currently Deno.test(() => {}) overload is not supported - there must be some test function name to register a function. I'm fine with exposing the name in test context.

@hyp3rflow Deno.mainModule requires full read permissions whereas import.meta.url doesn't require any permissions.

[hyp3rflow]

@bartlomieju Thanks for the explanation!

But I think there is no problem with using Deno.mainModule for snapshot testing because reading snapshot already requires read permission (although it will require permission only for snapshot, not test code).

Using Deno.mainModule, there is no need to provide the same argument to get the path of the test code for each time like below.

assertSnapshot(import.meta.url, obj); // We don't need to provide `import.meta.url`!

[bartlomieju]

@hyp3rflow that's a good point, I'm not sure how the API would look like. I'm open to using Deno.mainModule() if that means there's less boilerplate. I think it will be reason about once we have some PR going

[bcheidemann]

@bartlomieju @hyp3rflow I would propose the following API given the above discussion:

assertSnapshot(name: string, obj: any): void

This would use Deno.mainModule to determine the output file path of the snapshot file.

@bartlomieju if you have no major objections to this then I will raise an initial PR that we can use as a basis for further discussion :)

[bartlomieju]

@bcheidemann please do

[hyp3rflow]

@bartlomieju @bcheidemann I opened the PR that exposes the test name on TestContext. #14007
I'll open the rest of the implementation for assertSnapshot on deno_std.

[bartlomieju]

@hyp3rflow @bcheidemann are you working together on this? I don't want to ask you to do duplicate work :)

[hyp3rflow]

@bartlomieju No, I'm working on this feature alone. Is it okay for me to work on this? :q

[bcheidemann]

@hyp3rflow @bcheidemann are you working together on this? I don't want to ask you to do duplicate work :)

I was working on this.. But seems like I'm not anymore o.O

[bcheidemann]

@hyp3rflow @bcheidemann are you working together on this? I don't want to ask you to do duplicate work :)

I am open to working together on this. I already made a start on this and looking at your implementation @hyp3rflow I think there is good stuff that we can take from both of our approaches.

Are you 100% on doing this alone?

[hyp3rflow]

@bcheidemann yes I'm doing this alone and I also agree with you. but I'm a little bit afraid that there is a timezone issue so maybe communication can be delayed sometimes.