ethereumjs-monorepo icon indicating copy to clipboard operation
ethereumjs-monorepo copied to clipboard

Update Vitest to v1 (New)

Open holgerd77 opened this issue 1 year ago • 1 comments

Follow-up respectively replacement for #3191 (will close)

This is a fresh take on the Vitest test runner update from v0.32.0 to a v1 version (version on PR submission was v1.2.2, see Vitest Releases), also following the work from @ScottyPoi to extract necessary test structure changes in #3266.

This is not necessarily to be immediately picked up but just to get an impression on "where we are". It might eventually make sense to other extractable pre-work and then rebase this PR here and do the Vitest update itself only at the very end of things.

PR should also serve as some discussion ground on some observations in between.

First thing I noted that Vitest now has excluded all Node.js primitives and one need to manually add vite-plugin-node-polyfills. Have done here now, needed to add this for events in Common.

This leads me to the question: how problematic are actually Node.js events regarding browser? 🤔 Is this something to address (we can take this over to the chat)?

holgerd77 avatar Feb 14 '24 11:02 holgerd77

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.88%. Comparing base (107660e) to head (6e3f28a).

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 82.12% <ø> (-3.62%) :arrow_down:
blockchain 90.92% <ø> (-0.70%) :arrow_down:
client 84.92% <ø> (+0.31%) :arrow_up:
common 94.05% <100.00%> (-4.39%) :arrow_down:
devp2p 81.36% <ø> (-0.76%) :arrow_down:
ethash ?
evm 73.02% <ø> (-0.70%) :arrow_down:
genesis 99.98% <ø> (ø)
statemanager 74.71% <ø> (-1.45%) :arrow_down:
trie 59.60% <ø> (-29.41%) :arrow_down:
tx 85.70% <ø> (-9.38%) :arrow_down:
util 81.32% <ø> (-1.85%) :arrow_down:
vm 61.56% <ø> (-17.82%) :arrow_down:
wallet 87.25% <ø> (-1.11%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Feb 14 '24 11:02 codecov[bot]

Have gone through and updates this based on current master. Everything passes (most of the time) locally. I still get seemingly random errors like this for browser tests so still need to research further on that front.

image

acolytec3 avatar Mar 29 '24 17:03 acolytec3

One interesting thing I found is that we do not need to alias events to eventemitter3 and that actually introduces new problems. Have removed that from my attempts for now.

acolytec3 avatar Mar 29 '24 17:03 acolytec3

So we're down to wallet tests failing because of timeouts (they pass locally but are really slow on CI) and then the random failures I described above where browser tests fail because of random " failed to load dynamically imported module

acolytec3 avatar Mar 30 '24 10:03 acolytec3