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.

Sign up for GitHub

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

On-demand testing for VM State and Blockchain #951

Merged
merged 5 commits into from
Nov 17, 2020
Merged

On-demand testing for VM State and Blockchain #951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
342319b
ci: on-demand testing for VM State and Blockchain
evertonfraga Nov 12, 2020
40bdd4f
tests: moved ethereum-tests submodule to packages folder
holgerd77 Nov 17, 2020
3fb8eb5
tests: checked out ethereumjs-testing v1.3.3 ethereum/tests state on …
holgerd77 Nov 17, 2020
732ea8b
vm, tx: fixed test loader submodule paths
holgerd77 Nov 17, 2020
c0dd7ee
monorepo: fixed VM vm-blockchain, vm-state-extended submodule checkout
holgerd77 Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 92 additions & 20 deletions .github/workflows/vm-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defaults:
working-directory: packages/vm

jobs:
test-vm-api:
vm-api:
runs-on: ubuntu-latest
steps:
- uses: actions/setup-node@v1
Expand Down Expand Up @@ -49,21 +49,13 @@ jobs:
- run: npm run test:API:browser
- run: npm run lint

test-vm-state:
vm-state:
runs-on: ubuntu-latest
strategy:
matrix:
fork: [
# 'Berlin',
'MuirGlacier',
'Istanbul',
# 'Petersburg',
# 'Constantinople',
# 'Byzantium',
# 'SpuriousDragon',
# 'TangerineWhistle',
# 'Homestead',
# 'Chainstart'
'Istanbul'
]
fail-fast: false
steps:
Expand Down Expand Up @@ -93,7 +85,49 @@ jobs:

- run: npm run test:state -- --fork=${{ matrix.fork }} --verify-test-amount-alltests

test-vm-blockchain:
vm-state-extended:
if: contains(join(github.event.pull_request.labels.*.name, ' '), 'Test all hardforks')
runs-on: ubuntu-latest
strategy:
matrix:
fork: [
'Berlin',
'Petersburg',
'Constantinople',
'Byzantium',
'SpuriousDragon',
'TangerineWhistle',
'Homestead',
'Chainstart'
]
fail-fast: false
steps:
- uses: actions/setup-node@v1
with:
node-version: 12.x
- uses: actions/checkout@v1
with:
submodules: recursive
- name: Dependency cache
uses: actions/cache@v2
id: cache
with:
key: VM-${{ runner.os }}-12-${{ hashFiles('**/package-lock.json') }}
path: '**/node_modules'

# Installs root dependencies, ignoring Bootstrap All script.
# Bootstraps the current package only
- run: npm install --ignore-scripts && npm run bootstrap:vm
if: steps.cache.outputs.cache-hit != 'true'
working-directory: ${{github.workspace}}

# Builds current package and the ones it depends from.
- run: npm run build:vm
working-directory: ${{github.workspace}}

- run: npm run test:state -- --fork=${{ matrix.fork }} --verify-test-amount-alltests

vm-blockchain:
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -102,15 +136,53 @@ jobs:

# Tests were splitted with --dir and --excludeDir to balance execution times below the 9min mark.
args: [
# '--fork=Berlin --expected-test-amount=33',
'--fork=Istanbul --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
'--fork=Istanbul --excludeDir=stTimeConsuming --expected-test-amount=19817',
# '--fork=Constantinople --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
# '--fork=Constantinople --excludeDir=stTimeConsuming --expected-test-amount=17193',
# '--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
# '--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=17174',
Comment on lines -110 to -111
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to keep these commented out here, or should they be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryanio they are OK to be removed. In fact, they were just moved to these lines: https://github.com/ethereumjs/ethereumjs-vm/pull/951/files/c13ce861a06ce6969e59ede971db755dc2490734#diff-8ac46f60fffb24cf4daf7b192aee8bcd313d6fba95b005aa644e40600739a713R173-R179

# '--fork=Homestead --expected-test-amount=6997',
# '--fork=Chainstart --expected-test-amount=4385'
'--fork=Istanbul --excludeDir=stTimeConsuming --expected-test-amount=19817'
]
fail-fast: false
steps:
- uses: actions/setup-node@v1
with:
node-version: 12.x
- uses: actions/checkout@v1
with:
submodules: recursive
- name: Dependency cache
uses: actions/cache@v2
id: cache
with:
key: VM-${{ runner.os }}-12-${{ hashFiles('**/package-lock.json') }}
path: '**/node_modules'

# Installs root dependencies, ignoring Bootstrap All script.
# Bootstraps the current package only
- run: npm install --ignore-scripts && npm run bootstrap:vm
if: steps.cache.outputs.cache-hit != 'true'
working-directory: ${{github.workspace}}

# Builds current package and the ones it depends from.
- run: npm run build:vm
working-directory: ${{github.workspace}}

- run: npm run test:blockchain -- ${{ matrix.args }}

vm-blockchain-extended:
if: contains(join(github.event.pull_request.labels.*.name, ' '), 'Test all hardforks')
Copy link
Contributor

Choose a reason for hiding this comment

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

coool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not use the entire label name type: Test all hardforks, because YAML libraries have trouble with character escaping. The colon was interpreted as a regular key: value, resulting in syntax errors. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, ok, good to know! and I noticed the label has a lowercase t for test.

runs-on: ubuntu-latest
strategy:
matrix:
# Args to pass to the tester. Note that some have splitted the slow tests and only
# running those: these are only on forks where that is applicable (see PR #489 for numbers on these)

# Tests were splitted with --dir and --excludeDir to balance execution times below the 9min mark.
args: [
'--fork=Berlin --expected-test-amount=33',
'--fork=Constantinople --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
'--fork=Constantinople --excludeDir=stTimeConsuming --expected-test-amount=17193',
'--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
'--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=17174',
'--fork=Homestead --expected-test-amount=6997',
'--fork=Chainstart --expected-test-amount=4385'
]
fail-fast: false
steps:
Expand Down
2 changes: 1 addition & 1 deletion .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[submodule "ethereum-tests"]
path = ethereum-tests
path = packages/ethereum-tests
url = https://github.com/ethereum/tests.git
branch = develop
1 change: 0 additions & 1 deletion ethereum-tests
Open in desktop
Submodule ethereum-tests deleted from bfcd07
1 change: 1 addition & 0 deletions packages/ethereum-tests
Open in desktop
Submodule ethereum-tests added at 66a55c
2 changes: 1 addition & 1 deletion packages/tx/test/testLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path')

const falsePredicate = () => false
// Load tests from git submodule
const defaultTestsPath = path.resolve('../../ethereum-tests')
const defaultTestsPath = path.resolve('../ethereum-tests')
/**
* Returns the list of test files matching the given parameters
* @param testType the test type (path segment)
Expand Down
2 changes: 1 addition & 1 deletion packages/vm/tests/testLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path')

const falsePredicate = () => false
// Load tests from git submodule
const defaultTestsPath = path.resolve('../../ethereum-tests')
const defaultTestsPath = path.resolve('../ethereum-tests')
/**
* Returns the list of test files matching the given parameters
* @param testType the test type (path segment)
Expand Down