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

Open
simoneb opened this issue May 31, 2023 · 6 comments
Open

Snapshot testing #48260

simoneb opened this issue May 31, 2023 · 6 comments
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.

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

No branches or pull requests

8 participants