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

Monorepo: Vitest Test Transition / ESM Part 2 #2764

Merged
merged 63 commits into from
Jun 16, 2023
Merged

Conversation

holgerd77
Copy link
Member

@holgerd77 holgerd77 commented Jun 8, 2023

Transition of Tape tests to Vitest, also ESM .js file reference integration.

First results look promising 🙂.

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #2764 (d25b168) into master (b21e8d2) will decrease coverage by 0.66%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block ?
blockchain ?
client 87.09% <ø> (+0.01%) ⬆️
common 97.26% <100.00%> (+0.26%) ⬆️
devp2p ?
ethash ?
rlp ∅ <ø> (?)
statemanager 86.35% <ø> (+0.07%) ⬆️
trie ?
tx 95.87% <ø> (+0.20%) ⬆️
util 82.46% <ø> (-1.33%) ⬇️

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

@holgerd77 holgerd77 force-pushed the esm-part-2-5-with-tests branch 3 times, most recently from 29b4b4c to ccb8fd3 Compare June 12, 2023 12:28
"docs:build": "typedoc --options typedoc.js",
"lint": "../../config/cli/lint.sh",
"lint:diff": "../../config/cli/lint-diff.sh",
"lint:fix": "../../config/cli/lint-fix.sh",
"prepublishOnly": "../../config/cli/prepublish.sh",
"tape": "tape -r ts-node/register",
"test": "npm run test:node && npm run test:browser",
"test:browser": "karma start karma.conf.js",
"test:node": "npm run tape -- ./test/*.spec.ts",
"test:browser": "npx vitest run --browser=chrome",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"test:browser": "npx vitest run --browser=chrome",
"test:browser": "npx vitest run --browser.name=chrome --browser.headless

@holgerd77 holgerd77 force-pushed the esm-part-2-5-with-tests branch from dfb4171 to 32c4d42 Compare June 13, 2023 09:53
@@ -14,10 +14,9 @@ import { BlockHeader } from '../src/header.js'
import { fakeExponential, getNumBlobs } from '../src/helpers.js'
import { Block } from '../src/index.js'

import * as gethGenesis from './testdata/4844-hardfork.json'
import gethGenesis from './testdata/4844-hardfork.json'
Copy link
Member Author

Choose a reason for hiding this comment

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

TBH: on these cases I always totally have no idea what version to take and what difference this makes. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

For this one, if I understand how Typescript works with ESM, a namespace import like this converts the JSON into an object with a default export so it imported it as { "default": [ blocksData ] } instead of [ blocksData ].

@holgerd77 holgerd77 force-pushed the esm-part-2-5-with-tests branch from 65b71ef to d20eaa2 Compare June 15, 2023 07:21
@holgerd77
Copy link
Member Author

Rebased this and added various lint fixes

acolytec3
acolytec3 previously approved these changes Jun 16, 2023
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

Very excited to merge this. Now, ethash please cooperate and pass so we can merge.

@acolytec3 acolytec3 merged commit 972c682 into master Jun 16, 2023
@holgerd77 holgerd77 deleted the esm-part-2-5-with-tests branch June 16, 2023 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants