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

git workflow evmone coverage script #503

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

git workflow evmone coverage script #503

wants to merge 2 commits into from

Conversation

winsvega
Copy link
Collaborator

@winsvega winsvega commented Apr 12, 2024

🗒️ Description

git job to measure coverage of ported tests
if a new line introduced to converted_ethereum_tests, the script will run coverage and make an artifact

demonstration: #504

What does it do:

Evmone docker image

The main script is in this repo: https://github.com/winsvega/evmtest_coverage
Using ./dcoverage.sh build command a docker image of evmone in coverage mode is built.
As well as the coverage software is installed in the docker container.
A sanpshot of the container is currently hosted at http://retesteth.ethdevops.io/release/evmone.tar

Coverage script

(requires root to assign generated file permissions)

The script accpets 2 arguments.
testBASE - folder with the tests we had before coverage
testPATCH - folder with the tests we generated after converting existed .json tests fillers into .py

sudo ./dcoverage.sh --base=testBASE --patch=testPATCH [--driver=native|retesteth]

The folder ./coverage/evmone_coverage.sh is mounted inside the docker and execute instructions with evmone.
it automatically sorts state tests and blockchain tests and runs them on evmone using retesteth (t8n interface). (can be changed to evmone native test tools using the flag --driver=native, note that the current image support only retesteth as of now)

The script produces coverage report of testBase, testPatch and a diff of testPatch on top of testBase
Thus we can see what changed after we swithed from testBase to testPatch. The coverage diff output log is written to
./coverage/difflog.txt
Finally ./check.sh analyzes the log file and if it has the lines or function paths that lost it's coverage in testPatch version of the tests, it yells an error.
The result information is uploaded as a git artifact by the workflow script, so we can see what got lost.

🔗 Related Issues

Feauture

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@winsvega winsvega changed the base branch from eip1153 to main April 12, 2024 12:38
@winsvega winsvega requested a review from marioevz April 12, 2024 12:39
@winsvega winsvega added the type:test Type: Test label Apr 12, 2024
@winsvega winsvega added type:feat type: Feature scope:pytest Scope: Pytest plugins labels Apr 23, 2024
@winsvega winsvega force-pushed the coverage branch 3 times, most recently from f9e452f to c55a0ec Compare April 24, 2024 08:58
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Some comments.

Comment on lines 21 to 25
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }} # Checks out the PR branch
fetch-depth: 0 # Necessary to fetch all history for diff
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }} # Checks out the PR branch
fetch-depth: 0 # Necessary to fetch all history for diff
- name: Checkout code
uses: actions/checkout@v3

I think this action by default checks out the current PR's branch, so the ref is unnecessary.
Also, we seem to be doing this twice at the start of the steps? (Line 12).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there must be 2 separate steps. one is to fetch branches, and enother one to restore cache.
never used the git cache in this context, but it supposed to keep the docker file and only update it when the hash file is changed.

.github/workflows/coverage.yaml Outdated Show resolved Hide resolved
.github/workflows/coverage.yaml Outdated Show resolved Hide resolved
@winsvega winsvega requested a review from marioevz May 6, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:pytest Scope: Pytest plugins type:feat type: Feature type:test Type: Test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants