这是indexloc提供的服务,不要输入任何密码
Skip to content

Revamp snapshot tests #2351

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

Merged
merged 12 commits into from
Oct 28, 2022
Merged

Revamp snapshot tests #2351

merged 12 commits into from
Oct 28, 2022

Conversation

jridgewell
Copy link
Contributor

This fixes quite a few warts with out snapshot tests:

  1. Implements good diffing via similar crate.
    • It'll look just like git diffs now.
  2. Removes the HMR chunk item source maps
    • So, you'll only see 1 source map per emitted chunk file, not 1 per item in the chunk
  3. Makes source map files have a consistent fingerprint path
    • Every time the associated JS file contents changed, the .map file is renamed, making git show it as a new/deleted file instead of a changed one.
  4. Extracts snapshot tests into a separate crate
    • Due to a quirk in rust build.rs, we had to rebuild the turbopack crate whenever a test file changed
    • This slowed down the next test run, because everything that depends on turbopack (read, everything) needed to recompile
    • This slowed down the next build, for the same reason
    • Now, only the separate turbopack-tests crate rebuilds, and everything else stays constant

@jridgewell jridgewell requested a review from a team as a code owner October 26, 2022 01:58
@vercel
Copy link

vercel bot commented Oct 26, 2022

@jridgewell is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Ignored Deployments
Name Status Preview Updated
examples-basic-web ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:46PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:46PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:46PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:46PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:46PM (UTC)

@jridgewell jridgewell requested review from wbinnssmith and alexkirsz and removed request for nathanhammond and NicholasLYang October 27, 2022 21:57
let mut p = PathBuf::from(&path.await?.path);
if p.extension() == Some(OsStr::new("map")) {
p.set_extension("");
debug_assert_ne!(p.extension(), Some(OsStr::new("js")));
Copy link
Contributor

Choose a reason for hiding this comment

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

In what case is this possible, since we set the extension to "" just before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It recomputes afterwards, inheriting the next .abc123 that still remains. I think it's implemented like an rfind.

diff.unified_diff()
.context_radius(3)
.header("expected", "actual")
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps use eprintln instead of println in the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@ForsakenHarmony ForsakenHarmony left a comment

Choose a reason for hiding this comment

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

Any chance we could merge the snapshot output into a single file?

@jridgewell
Copy link
Contributor Author

Any chance we could merge the snapshot output into a single file?

Does this make it easier? I really dislike jests file format, because there's cruft alongside the snapshot outputs.

@ForsakenHarmony
Copy link
Contributor

Would make git diffs a bit nicer if we don't have 10 changed files for every snapshot test all the time

But that doesn't have to be in this PR

@sokra sokra merged commit e7815fd into vercel:main Oct 28, 2022
jridgewell added a commit to ahaoboy/turbo that referenced this pull request Oct 28, 2022
jridgewell added a commit that referenced this pull request Oct 28, 2022
* chore: typo

* Remove hot_module_replacement from snapshot tests

Re: #2351

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
* chore: typo

* Remove hot_module_replacement from snapshot tests

Re: vercel/turborepo#2351

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
* chore: typo

* Remove hot_module_replacement from snapshot tests

Re: vercel/turborepo#2351

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
* Disable HMR for snapshot tests

The only effect is that we don't emit source maps for the individual chunk items, reducing the total number of output files considerably.

* Extract snapshot tests into separate crate

* Strip annoying file hash fingerprints

* Reduce deps

* Update old references to turbopack/test/snapshot

* Move node_modules out of snapshot dir

* Only un-fingerprint EcmascriptChunkSourceMapAssetVc source maps

* Fix gitignore

* Remove dbg

* TAPLO

* Apply suggestions from code review

Co-authored-by: Alex Kirszenberg <a.kirszenberg@gmail.com>

* Use eprintln

Co-authored-by: Alex Kirszenberg <a.kirszenberg@gmail.com>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
* chore: typo

* Remove hot_module_replacement from snapshot tests

Re: vercel/turborepo#2351

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
* Disable HMR for snapshot tests

The only effect is that we don't emit source maps for the individual chunk items, reducing the total number of output files considerably.

* Extract snapshot tests into separate crate

* Strip annoying file hash fingerprints

* Reduce deps

* Update old references to turbopack/test/snapshot

* Move node_modules out of snapshot dir

* Only un-fingerprint EcmascriptChunkSourceMapAssetVc source maps

* Fix gitignore

* Remove dbg

* TAPLO

* Apply suggestions from code review

Co-authored-by: Alex Kirszenberg <a.kirszenberg@gmail.com>

* Use eprintln

Co-authored-by: Alex Kirszenberg <a.kirszenberg@gmail.com>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
* chore: typo

* Remove hot_module_replacement from snapshot tests

Re: vercel/turborepo#2351

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants