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

eofparse: Return number of errors #873

Merged
merged 1 commit into from
Apr 26, 2024
Merged

eofparse: Return number of errors #873

merged 1 commit into from
Apr 26, 2024

Conversation

chfast
Copy link
Member

@chfast chfast commented Apr 24, 2024

In the evmone-eofparse tool exit with the number of invalid EOFs encountered.

Also add some basic integration tests.

Fixes #872

Copy link

codecov bot commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 98.39%. Comparing base (1aa5539) to head (63bb6b5).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #873   +/-   ##
=======================================
  Coverage   98.38%   98.39%           
=======================================
  Files         129      130    +1     
  Lines       15578    15615   +37     
=======================================
+ Hits        15327    15364   +37     
  Misses        251      251           
Flag Coverage Δ
statetests 27.78% <ø> (ø)
statetests-silkpre 18.76% <ø> (ø)
unittests 94.21% <80.00%> (+0.01%) ⬆️

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

Files Coverage Δ
test/eofparse/eofparse.cpp 89.18% <80.00%> (ø)

... and 1 file with indirect coverage changes

@chfast chfast added the tests Testing infrastructure label Apr 24, 2024
@chfast chfast force-pushed the eofparse branch 7 times, most recently from cc4ca1e to bdaa2bd Compare April 24, 2024 14:19
@chfast chfast requested review from gumb0, pdobacz and rodiazet April 26, 2024 07:47
Copy link
Collaborator

@pdobacz pdobacz left a comment

Choose a reason for hiding this comment

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

lgtm, one idea for a test

add_test(NAME ${PREFIX}/eof_version_unknown COMMAND sh -c "echo EF00.FF | ${CROSSCOMPILING_EMULATOR} $<TARGET_FILE:evmone-eofparse>")
set_tests_properties(${PREFIX}/eof_version_unknown PROPERTIES PASS_REGULAR_EXPRESSION "err: eof_version_unknown")

add_test(NAME ${PREFIX}/invalid_hex COMMAND sh -c "echo gaga | ${CROSSCOMPILING_EMULATOR} $<TARGET_FILE:evmone-eofparse>")
Copy link
Collaborator

Choose a reason for hiding this comment

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

to test the new behavior, would a command like echo <code1> \n <invalid_code2> \n <invalid_code3> | eofparse || echo $? and then assert that 2 got printed in the end work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I didn't know how to test the exit code...

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 Maybe something like <eofparse> > /dev/null; echo $? would be more robust, just assert result is exactly 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did exactly this. Hopefully, there will be no issues with shell on different OSes.

@chfast chfast force-pushed the eofparse branch 5 times, most recently from b69d978 to 1e07c7f Compare April 26, 2024 13:11
In the evmone-eofparse tool exit with the number of invalid EOFs
encountered.

Also add some basic integration tests.
@chfast chfast merged commit b6d9a11 into master Apr 26, 2024
22 of 23 checks passed
@chfast chfast deleted the eofparse branch April 26, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Testing infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

evmone-eofparse does not return an error code
2 participants