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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jest-snapshot] Custom Snapshot Directories confuse the Obsolete Snapshot Logger #7257

Closed
fbartho opened this issue Oct 24, 2018 · 12 comments
Closed

Comments

@fbartho
Copy link

fbartho commented Oct 24, 2018

馃悰 Bug Report

When using the new snapshot resolver feature: #6143, the "Obsolete" Snapshot checker is deciding that all my snapshots are obsolete after a test run.

To Reproduce

  1. Using a simple project, configure a snapshot resolver to move your snapshot files to a different directory-structure.
  2. Run yarn test notice all your tests pass, and snapshots are in their new location
  3. Notice that it says 45 snapshot files obsolete from 45 test suites.
  4. Run jest -u to remove obsolete snapshots, notice that all your snapshots are gone.

Expected behavior

I would expect that the snapshot files that just got written to wouldn't be considered instantly obsolete.

Speculation: It looks like the custom snapshotResolver isn't invoked when analyzing obsolete snapshots, so Jest doesn't know that these files were actually written to, or something else is weird.

Link to repl or repo (highly encouraged)

I haven't yet figured out how to do Snapshot Testing in repl.it, will upload a repo shortly if I can't find an example.

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS High Sierra 10.13.6
    CPU: x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
  Binaries:
    Node: 10.10.0 - /usr/local/bin/node
    Yarn: 1.9.4 - /usr/local/bin/yarn
    npm: 6.0.0 - /usr/local/bin/npm
  npmPackages:
    @types/jest: 23.x => 23.3.7 
    jest: ^24.0.0-alpha.1 => 24.0.0-alpha.1 
@fbartho
Copy link
Author

fbartho commented Oct 24, 2018

My SnapshotResolver:

const path = require("path");

const rootDir = path.resolve(__dirname, "..", "..");
const distDir = path.join(rootDir, "dist");
const srcDir = path.join(rootDir, "src");

// const debug = console.debug;
const debug = () => {
	return;
};

module.exports = {
	/** resolves from test to snapshot path */
	resolveSnapshotPath: (testPath, snapshotExtension) => {
		const result =
			testPath
				.replace("__tests__", "__tests__/__snapshots__")
				.replace(distDir, rootDir) + snapshotExtension;
		debug("testPath", testPath, snapshotExtension, result);
		return result;
	},

	/** resolves from snapshot to test path */
	resolveTestPath: (snapshotFilePath, snapshotExtension) => {
		const result = snapshotFilePath
			.replace("__snapshots__/", "")
			.replace(distDir, rootDir)
			.slice(0, -snapshotExtension.length);
		debug("snapshotFilePath", snapshotFilePath, snapshotExtension, result);
		return result;
	},
};

@fbartho
Copy link
Author

fbartho commented Oct 24, 2018

I wonder if the root cause is related to #6773 ?

@SimenB
Copy link
Member

SimenB commented Oct 24, 2018

@viddo would you be able to investigate this?


I haven't yet figured out how to do Snapshot Testing in repl.it, will upload a repo shortly if I can't find an example.

You might be able to use code sandbox, but a repository might be even easier

@fbartho
Copy link
Author

fbartho commented Oct 24, 2018

@SimenB This Issue suggests that Snapshot testing in CodeSandbox might happen in 3.0, but it's still in progress: codesandbox/codesandbox-client#513

So I guess I'll do a repo at some point, then!

@fbartho
Copy link
Author

fbartho commented Nov 6, 2018

I have a repro, packaging it up shortly!

@fbartho
Copy link
Author

fbartho commented Nov 6, 2018

Reproduction provided here! https://github.com/fbartho/jest-snapshot-custom-paths-bug-reproduction

@SimenB -- please let me know if there's anything further I can do to help!

I've verified it occurs on alpha.4

npx envinfo --preset jest
npx: installed 1 in 2.021s

  System:
    OS: macOS 10.14
    CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
  Binaries:
    Node: 10.12.0 - ~/.nvm/versions/node/v10.12.0/bin/node
    Yarn: 1.12.1 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v10.12.0/bin/npm
  npmPackages:
    jest: 24.0.0-alpha.4 => 24.0.0-alpha.4 

@fbartho
Copy link
Author

fbartho commented Nov 6, 2018

@facebook-github-bot label 馃悰 Bug

@fbartho
Copy link
Author

fbartho commented Nov 6, 2018

@facebook-github-bot label Has Repro

@viddo
Copy link
Contributor

viddo commented Nov 11, 2018

First, sorry for late reply, have had some busy weeks.

The bug itself is in the custom resolver it turns out, I sent a PR on the reproduce repo (thanks for setting that up BTW), see https://github.com/fbartho/jest-snapshot-custom-paths-bug-reproduction/pull/1/files

We did add a "preflight" check with the intention of catching prevent this very scenario from happening, but as can be seen in this reproduce it's quite obvious it's insufficient. 馃槥

A proper sanity check will need to do know exactly what kind of transformation the customer resolver is doing to validate it (in this particular case remapping src<->dist). The simplest and most straight forward idea I can think of would be to require implementation to also provide an example path to use instead of the hardcoded some-path/__tests__/snapshot_resolver.test.js. I can think of more involved and complicated ways to validate the resolver but more hesitant about that approach. Ideas or thoughts?

edit: #7356 is the proposed fix

@fbartho
Copy link
Author

fbartho commented Nov 11, 2018

Issue resolved, thanks @viddo! and #7356 would have prevented me from making that original mistake. Closing issue!

@vicasas
Copy link

vicasas commented Apr 2, 2021

This error still continues. I'm with the latest version of jest.

This is my configuration:

module.exports = {
  // resolves from test to snapshot path
  resolveSnapshotPath: (testPath, snapshotExtension) =>
    testPath
      .replace('__tests__', '__snapshots__')
      .replace('components', '__snapshots__') + snapshotExtension,

  // resolves from snapshot to test path
  resolveTestPath: (snapshotFilePath, snapshotExtension) =>
    snapshotFilePath
      .replace('__snapshots__', '__tests__')
      .replace('__snapshots__', 'components')
      .slice(0, -snapshotExtension.length),

  // Example test path, used for preflight consistency check of the implementation above
  testPathForConsistencyCheck: 'some/__tests__/example.test.js',
}

image

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants