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

Update vitest #3369

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Update vitest #3369

wants to merge 34 commits into from

Conversation

acolytec3
Copy link
Contributor

@acolytec3 acolytec3 commented Apr 24, 2024

Updates vitest and vitest/browser to latest versions to resolve a peer dep mismatch

@acolytec3 acolytec3 added the dependencies Pull requests that update a dependency file label Apr 24, 2024
import { nodePolyfills } from 'vite-plugin-node-polyfills'
import wasm from 'vite-plugin-wasm'

const config = defineConfig({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const config = defineConfig({
// https://vitest.dev/config/
const config = defineConfig({

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roninjin10 do you have any insights on why an arbitrary browser test script seems to always fail on CI? All of our browser tests pass locally but when they run on CI, we get these failed to resolve dynamically imported module... errors. I've tried all manner of config settings trying to remove parallel test runs and isolated environments, etc, with no success. At this point, I haven't come up with a reproducible "minimal" build to point the vitest devs at so have hesitated to open an issue there (though may do so soon)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@acolytec3 try setting VITEST_BROWSER_DEBUG=true environment variable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw if the debug logging doesn't provide any insight let me know I'd be happy to look at this

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 just keeps doing this sort of thing - https://github.com/ethereumjs/ethereumjs-monorepo/actions/runs/8943487337/job/24568309037#step:9:74

Debug logging doesn't seem to tell me anything more. It's not always the tx test run that fails either. I can comment out tx and another one will fail the same way (or I can tell vitest to skip the one test file that's not being loaded and it will still fail on tx with a different file having no tests fail). Very weird.

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 seems to have started after vitest v1.5.1 was released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't have a quick instinct on it, I'll just open an issue on vitest and see if they're willing to look at our vitest config since I don't know how else to reproduce it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

. It's not always the tx test run that fails either. I can comment out tx and another one will fail the same way (or I can tell vitest to skip the one test file that's not being loaded and it will still fail on tx with a different file having no tests fail). Very weird.

oh that is really wierd and definitely smells like an internal race condition bug in vitest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my instinct as well. Will report it and see if they're willing to help.

@acolytec3 acolytec3 linked an issue May 6, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vitest type issue (WeakKey type)
2 participants