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

Snapshot testing #48260

Closed
simoneb opened this issue May 31, 2023 · 6 comments · Fixed by #53169
Closed

Snapshot testing #48260

simoneb opened this issue May 31, 2023 · 6 comments · Fixed by #53169
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@simoneb
Copy link
Contributor

simoneb commented May 31, 2023

What is the problem this feature will solve?

It would be useful to have snapshot testing as part of the built-in test runner or assertion library.

I am aware of earlier work on this, specifically the inclusion and then removal of assert.snapshot as per #44466 and #46112.

I'm wondering if there's a consensus that snapshot testing should not live in Node core or if you would be open to an implementation without the shortcomings of the earlier one.

What is the feature you are proposing to solve the problem?

I would propose an implementation in Node core, which would play well with the built-in test runner

What alternatives have you considered?

The only real alternative at the moment would be using a different testing framework. I haven't considered whether snapshot testing could be implemented as a standalone library though.

@simoneb simoneb added the feature request Issues that request new features to be added to Node.js. label May 31, 2023
@cjihrig
Copy link
Contributor

cjihrig commented May 31, 2023

I would definitely sign off on a new implementation of assert.snapshot() if it addressed the issues raised with the previous implementation.

@MoLow
Copy link
Member

MoLow commented May 31, 2023

I am +1.
I have added common.assertSnapshot to the internal node test suite - this way we can possibly use that as a foundation

@tniessen
Copy link
Member

It would be great to clarify expectations and requirements before we implement any API. assert.snapshot() was deeply flawed in many ways. common.assertSnapshot() is a much simpler internal implementation that may be sufficient for some simple use cases.

For example, would users expect snapshot testing to support data types other than strings?

@cjihrig cjihrig added assert Issues and PRs related to the assert subsystem. and removed test_runner labels Jun 10, 2023
@MrHBS
Copy link

MrHBS commented Nov 5, 2023

I would just see how other test runners, like Vitest and Jest, implement snapshots and do the same.

@hulkish
Copy link

hulkish commented Dec 5, 2023

Would really like to see this added to node +1

@JakobJingleheimer
Copy link
Contributor

I think this does not belong in node core because it's too specific. But I think we should facilitate it. In order to do so, I believe we need to expose some additional properties on TestContext:

  • file (à la the various events emitted)
  • fullName (the concatenated names of self and all ancestors)

Then someone (such as me—I just started a basic package for it to include/reference from the nodejs.org Learn article I'm working on) has sufficient data available to do this.

cjihrig added a commit to cjihrig/node that referenced this issue May 27, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

Fixes: nodejs#48260
cjihrig added a commit to cjihrig/node that referenced this issue May 28, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

Fixes: nodejs#48260
cjihrig added a commit to cjihrig/node that referenced this issue May 29, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

Fixes: nodejs#48260
cjihrig added a commit to cjihrig/node that referenced this issue May 29, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

Fixes: nodejs#48260
targos pushed a commit that referenced this issue Jun 1, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

PR-URL: #53169
Fixes: #48260
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this issue Jun 1, 2024
This commit adds a t.assert.snapshot() method that implements
snapshot testing. Serialization uses JSON.stringify() by default,
but users can configure the serialization to meet their needs.

PR-URL: nodejs#53169
Fixes: nodejs#48260
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.
Projects
Status: Pending Triage
Development

Successfully merging a pull request may close this issue.

8 participants