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

Integrated tests with Lerna (does not include Block for now) #734

Merged
merged 9 commits into from
Apr 30, 2020

Conversation

evertonfraga
Copy link
Contributor

Down the road of the monorepo transition, this PR introduces integrated module testing, powered by Lerna bootstrap command, with some additional configuration. This enables us to detect possible disruption of downstream libraries before releasing.

The monorepo codebase had Block in a non-production state (3.0.0), that wasn't integrated to the other packages at all, making this very transition a bit cumbersome.

Note to reviewers: I advise review looking at one commit at a time. It contains:

  1. Reverts Block code to 2.2.2. This is an inflated commit (12d503d)
  2. Removes NodeJS 8.x from tests (soon we'll include node 14) (e8e3201)
  3. Removes specific code and scripts for running tests with ts-node with linked packages, which will come at a later stage. (74890a2) and (8d5f3c0)
  4. Configures test coverage using the same standard as other ethereumjs-VM packages(ca04215)
  5. Puts lerna commands in place (75b0479).

This commit (75b0479) also disables node_modules cache on GitHub Actions test suite. They consider shallow paths (./node_modules), and generic caching keys, which was 100% correct before the monorepo transition. Related: #730

This PR replaces #725 and should be merged to master instead.

@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #734 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #734      +/-   ##
==========================================
- Coverage   91.72%   91.71%   -0.01%     
==========================================
  Files          47       46       -1     
  Lines        3008     3005       -3     
  Branches      468      468              
==========================================
- Hits         2759     2756       -3     
  Misses        151      151              
  Partials       98       98              
Flag Coverage Δ
#account 94.11% <ø> (ø)
#block 88.36% <ø> (ø)
#blockchain 89.15% <ø> (ø)
#common 94.26% <ø> (-0.11%) ⬇️
#tx 94.11% <ø> (ø)
#vm 92.53% <ø> (ø)
Impacted Files Coverage Δ
packages/common/src/genesisStates/index.ts 100.00% <0.00%> (ø)
packages/common/src/hardforks/index.ts

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ff1264...571bb4a. Read the comment docs.

@evertonfraga evertonfraga mentioned this pull request Apr 27, 2020
@holgerd77
Copy link
Member

This "revert Block code to 2.2.2" thing makes me a bit nervous since it is a pretty heavy and invasive step, is this really necessary to achieve what you want? How will you achieve to get these changes in again with a retained authorship and stuff like that? 🤔

@evertonfraga evertonfraga changed the title Integrated tests with Lerna Integrated tests with Lerna (do not include Block for now) Apr 29, 2020
@evertonfraga
Copy link
Contributor Author

evertonfraga commented Apr 29, 2020

You were right @holgerd77, and in the end I just wanted to verify that all the production packages worked well in this new scenario, no need for it to reach master. It did worked nicely, then I moved back to Block v3 source code.

After a surgical rebase, I removed the Revert Block to 2.2.2, along with some other Block-specific commits. I also edited the test: enabling Lerna for integrated tests, excluding the Lerna changes to Block, amending the commit message to test: enabling Lerna for integrated tests, all packages but Block.

These changes guarantees all packages (but Block) work well in conjunction, so we can finally tackle the Block v3 migration with more confidence.

@evertonfraga evertonfraga marked this pull request as ready for review April 29, 2020 20:12
- checkout
- run:
name: Installing dependencies
command: cd ~/project/packages/${CIRCLE_JOB} && npm install
Copy link
Contributor Author

@evertonfraga evertonfraga Apr 29, 2020

Choose a reason for hiding this comment

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

For this PR, Block won't use Lerna for linking packages. So in both CIs, we need to explicitly keep npm install for Block package.

"version": "0.0.0",
"command": {
"bootstrap": {
"ignore": "ethereumjs-block"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is important: it tells Lerna to ignore ethereumjs-block from lerna bootstrap commands. It will still be used on lerna run and lerna exec for the time being.

@evertonfraga evertonfraga changed the title Integrated tests with Lerna (do not include Block for now) Integrated tests with Lerna (does not include Block for now) Apr 29, 2020
.nyc_output
docs
Copy link
Contributor

Choose a reason for hiding this comment

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

Great, would love to add this to our global prettier config in the future 👍

Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for all your hard work on this!

@evertonfraga evertonfraga merged commit 65c9620 into master Apr 30, 2020
@evertonfraga evertonfraga deleted the integration-tests-block-2.2.2 branch April 30, 2020 02:41
@holgerd77
Copy link
Member

👍

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.

3 participants