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

fix: use correct source maps in stacktrace #2027

Merged
merged 17 commits into from Oct 7, 2022

Conversation

haikyuu
Copy link
Contributor

@haikyuu haikyuu commented Sep 12, 2022

use vitenode server to get a fetch result instead of a transform result.
And then get the sourcemap from it.

This fixes #2024

use vitenode server to get a fetch result instead of a transform result.
And then get the sourcemap from it.
Copy link
Member

@sheremet-va sheremet-va left a comment

Choose a reason for hiding this comment

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

Can you add a test for this, please?

packages/vitest/src/utils/source-map.ts Outdated Show resolved Hide resolved
@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 12, 2022

Can you add a test for this, please?

Sure, is there a test example for errors/stacktraces I can check? @sheremet-va

@sheremet-va
Copy link
Member

Can you add a test for this, please?

Sure, is there a test example for errors/stacktraces I can check? @sheremet-va

To be honest, I don't know.

@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 12, 2022

I found some failure tests I will start from https://github.com/vitest-dev/vitest/blob/main/test/fails/test/runner.test.ts#L0-L1

Then I'll create a couple of tests:

  • Test that js and ts test files have correct sourcemaps
  • Test that files transformed with a plugin has correct sourcemaps
    • I will add a simple plugin for .imba files in the vite.config.js. The plugin is dumb and always returns the same result + sourcemap for the .imba file in the test.

@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 12, 2022

The test fails because of this error https://github.com/vitest-dev/vitest/runs/8301687675?check_suite_focus=true#step:7:668

Error: Failed to parse source for import analysis because the content contains invalid JS syntax. 
You may need to install appropriate plugins to handle the .imba file format, 
or if it's an asset, add \\"**/*.imba\\" to \`assetsInclude\` in your configuration.

I used my complete plugin to generate the snapshot, but I created a small version of the plugin just for the test. But I don't understand why Vite is complaining about this error. I'd use some help here 🆘

@sheremet-va
Copy link
Member

I used my complete plugin to generate the snapshot, but I created a small version of the plugin just for the test. But I don't understand why Vite is complaining about this error. I'd use some help here 🆘

Try adding enforce: 'pre'

@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 12, 2022

Thanks @sheremet-va I was including extra quotes because I copied the value from the debugger. Fixed now :)

@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 12, 2022

@sheremet-va The tests are failing because of another error in https://github.com/vitest-dev/vitest/blob/main/test/fails/fixtures/timeout.test.ts#L4 that's not related to my PR

 Snapshots  1 failed
  - Expected   ""Error: Test timed out in 10ms.""
  + Received   ""Error: Hook timed out in 10ms.""

Tests were green before I merged main https://github.com/vitest-dev/vitest/runs/8302216560?check_suite_focus=true

Seems like there is some flakiness going on with these tests

test/stacktraces/test/runner.test.ts Outdated Show resolved Hide resolved
@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 13, 2022

@sheremet-va now one test which is not related to my changes seems flaky: https://github.com/vitest-dev/vitest/blob/main/test/fails/test/runner.test.ts

  - Expected   ""Error: Test timed out in 10ms.""
  + Received   ""Error: Hook timed out in 10ms.""

This is related to #2046 and I fixed it by renaming the file

@haikyuu
Copy link
Contributor Author

haikyuu commented Sep 15, 2022

@sheremet-va can you take a look again? I fixed the previous failing test. But I'm not sure why single threaded tests are failing in Ubuntu?

@sheremet-va sheremet-va changed the title [vitest] use correct source maps in stacktrace fix: use correct source maps in stacktrace Oct 7, 2022
@sheremet-va sheremet-va merged commit d1919a0 into vitest-dev:main Oct 7, 2022
AlexKMarshall added a commit to AlexKMarshall/necromunda-turbo-old that referenced this pull request Oct 7, 2022
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@vitest/coverage-c8](https://togithub.com/vitest-dev/vitest) |
[`0.23.4` ->
`0.24.0`](https://renovatebot.com/diffs/npm/@vitest%2fcoverage-c8/0.23.4/0.24.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@vitest%2fcoverage-c8/0.24.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@vitest%2fcoverage-c8/0.24.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@vitest%2fcoverage-c8/0.24.0/compatibility-slim/0.23.4)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@vitest%2fcoverage-c8/0.24.0/confidence-slim/0.23.4)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>vitest-dev/vitest</summary>

###
[`v0.24.0`](https://togithub.com/vitest-dev/vitest/releases/tag/v0.24.0)

[Compare
Source](https://togithub.com/vitest-dev/vitest/compare/v0.23.4...v0.24.0)

#####    🚨 Breaking Changes

- Use type module (revert
[#&#8203;1411](https://togithub.com/vitest-dev/vitest/issues/1411))  - 
by [@&#8203;bluwy](https://togithub.com/bluwy) and
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#1465
- Drop support for Vite 2  -  by
[@&#8203;antfu](https://togithub.com/antfu) and
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#1928

#####    🚀 Features

- **benchmark**: Todo mode  -  by
[@&#8203;Aslemammad](https://togithub.com/Aslemammad) in
[vitest-dev/vitest#2057
- **inline-snapshot**: Support comment  -  by
[@&#8203;azaleta](https://togithub.com/azaleta) in
[vitest-dev/vitest#2077

#####    🐞 Bug Fixes

- Run related test, even if test doesn't have dependencies  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#2043
- Check for asymmetricMatch before accessing  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(75719)</samp>](https://togithub.com/vitest-dev/vitest/commit/757199a6)
- Check hook teardown return type, closes
[#&#8203;2092](https://togithub.com/vitest-dev/vitest/issues/2092)  - 
by [@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(cba3f)</samp>](https://togithub.com/vitest-dev/vitest/commit/cba3ff09)
- Don't stop watch mode, if non-object error is thrown, close
[#&#8203;2106](https://togithub.com/vitest-dev/vitest/issues/2106)  - 
by [@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(bd677)</samp>](https://togithub.com/vitest-dev/vitest/commit/bd677017)
- Use correct source maps in stacktrace  -  by
[@&#8203;haikyuu](https://togithub.com/haikyuu) in
[vitest-dev/vitest#2027
- Import CustomEventMap from vite for vite-node  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#2124
- **jsdom**: Use jsdom Blob instead of Node, if jsdom is enabled  -  by
[@&#8203;ChpShy](https://togithub.com/ChpShy) in
[vitest-dev/vitest#2086

#####     [View changes on
GitHub](https://togithub.com/vitest-dev/vitest/compare/v0.23.4...v0.24.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click
this checkbox.

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/AlexKMarshall/necromunda-turbo).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMjIuMyIsInVwZGF0ZWRJblZlciI6IjMyLjIyMi4zIn0=-->
AlexKMarshall added a commit to AlexKMarshall/necromunda-turbo-old that referenced this pull request Oct 7, 2022
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [vitest](https://togithub.com/vitest-dev/vitest) | [`0.23.4` ->
`0.24.0`](https://renovatebot.com/diffs/npm/vitest/0.23.4/0.24.0) |
[![age](https://badges.renovateapi.com/packages/npm/vitest/0.24.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/vitest/0.24.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/vitest/0.24.0/compatibility-slim/0.23.4)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/vitest/0.24.0/confidence-slim/0.23.4)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>vitest-dev/vitest</summary>

###
[`v0.24.0`](https://togithub.com/vitest-dev/vitest/releases/tag/v0.24.0)

[Compare
Source](https://togithub.com/vitest-dev/vitest/compare/v0.23.4...v0.24.0)

#####    🚨 Breaking Changes

- Use type module (revert
[#&#8203;1411](https://togithub.com/vitest-dev/vitest/issues/1411))  - 
by [@&#8203;bluwy](https://togithub.com/bluwy) and
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#1465
- Drop support for Vite 2  -  by
[@&#8203;antfu](https://togithub.com/antfu) and
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#1928

#####    🚀 Features

- **benchmark**: Todo mode  -  by
[@&#8203;Aslemammad](https://togithub.com/Aslemammad) in
[vitest-dev/vitest#2057
- **inline-snapshot**: Support comment  -  by
[@&#8203;azaleta](https://togithub.com/azaleta) in
[vitest-dev/vitest#2077

#####    🐞 Bug Fixes

- Run related test, even if test doesn't have dependencies  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#2043
- Check for asymmetricMatch before accessing  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(75719)</samp>](https://togithub.com/vitest-dev/vitest/commit/757199a6)
- Check hook teardown return type, closes
[#&#8203;2092](https://togithub.com/vitest-dev/vitest/issues/2092)  - 
by [@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(cba3f)</samp>](https://togithub.com/vitest-dev/vitest/commit/cba3ff09)
- Don't stop watch mode, if non-object error is thrown, close
[#&#8203;2106](https://togithub.com/vitest-dev/vitest/issues/2106)  - 
by [@&#8203;sheremet-va](https://togithub.com/sheremet-va)
[<samp>(bd677)</samp>](https://togithub.com/vitest-dev/vitest/commit/bd677017)
- Use correct source maps in stacktrace  -  by
[@&#8203;haikyuu](https://togithub.com/haikyuu) in
[vitest-dev/vitest#2027
- Import CustomEventMap from vite for vite-node  -  by
[@&#8203;sheremet-va](https://togithub.com/sheremet-va) in
[vitest-dev/vitest#2124
- **jsdom**: Use jsdom Blob instead of Node, if jsdom is enabled  -  by
[@&#8203;ChpShy](https://togithub.com/ChpShy) in
[vitest-dev/vitest#2086

#####     [View changes on
GitHub](https://togithub.com/vitest-dev/vitest/compare/v0.23.4...v0.24.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click
this checkbox.

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/AlexKMarshall/necromunda-turbo).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMjIuMyIsInVwZGF0ZWRJblZlciI6IjMyLjIyMi4zIn0=-->
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.

Error shown with line:column number that are off (source map not used)
2 participants