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

Jump to bottom

test: Add call depth unit tests #757

Merged
merged 2 commits into from
Mar 19, 2021
Merged

test: Add call depth unit tests #757

merged 2 commits into from
Mar 19, 2021

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Mar 12, 2021 •
edited by axic
Loading

Depends on #756.

These test inspect the behavior of call depth limiting.

This duplicates a number of checks from "execute_call". Some tests from there may be removed if desired.

This implements test cases from https://notes.ethereum.org/@chfast/fizzy-call-depth.

@codecov
Copy link

codecov bot commented Mar 12, 2021 •
edited
Loading

Codecov Report

Merging #757 (73d935e) into master (e9b24a7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           master     #757    +/-   ##
========================================
  Coverage   99.25%   99.26%            
========================================
  Files          76       77     +1     
  Lines       11567    11718   +151     
========================================
+ Hits        11481    11632   +151     
  Misses         86       86
Flag Coverage Δ
rust 99.86% <ø> (ø)
spectests 90.55% <ø> (ø)
unittests 99.22% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
test/unittests/execute_call_depth_test.cpp 100.00% <100.00%> (ø)
test/unittests/execute_call_test.cpp 100.00% <100.00%> (ø)

@chfast chfast force-pushed the depth_tests branch 5 times, most recently from 92bb9af to d42c6b4 Compare March 16, 2021 13:31
@chfast
Copy link
Collaborator Author

chfast commented Mar 16, 2021

This duplicates a number of checks from "execute_call". Some tests from there may be removed if desired.

They are all "replaced" in the current version.

@chfast chfast requested review from axic and gumb0 March 16, 2021 14:05
@chfast chfast marked this pull request as ready for review March 16, 2021 14:05
@gumb0
Copy link
Collaborator

gumb0 commented Mar 16, 2021

Add new file to CI wat2wasm4tests check.

@gumb0
Copy link
Collaborator

gumb0 commented Mar 16, 2021

Add new file to CI wat2wasm4tests check.

Ah nevermind, we run it for all test files already.

@gumb0
Copy link
Collaborator

gumb0 commented Mar 16, 2021

The new tests look ok to me. I didn't check very thoroughly if they cover everything of the old tests.

axic
axic reviewed Mar 18, 2021
View reviewed changes
lib/fizzy/execute.hpp Outdated Show resolved Hide resolved
axic
axic reviewed Mar 18, 2021
View reviewed changes
lib/fizzy/execute.hpp Outdated Show resolved Hide resolved
axic
axic reviewed Mar 18, 2021
View reviewed changes
bindings/rust/src/lib.rs Outdated Show resolved Hide resolved
@axic axic mentioned this pull request Mar 18, 2021
Merged
axic
axic reviewed Mar 18, 2021
View reviewed changes
test/unittests/execute_call_depth_test.cpp

/// Infinite recursion

TEST(execute_call_depth, execute_internal_function_infinite_recursion)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, can you add a case similar to this where recursion starts at the start function? Generally also cases where start function is present and calls other things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.

Base automatically changed from depth_param to master March 19, 2021 16:43
chfast added 2 commits March 19, 2021 18:41
@chfast
test: Add call depth unit tests
bac0bed 
These test inspect the behavior of call depth limiting.
@chfast
test: Migrate host function recursion limit tests
73d935e 
This reworks tests which have host function recursion and moves them
to the execute_call_depth suite.
@chfast chfast merged commit b7d6de4 into master Mar 19, 2021
@chfast chfast deleted the depth_tests branch March 19, 2021 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axic axic axic approved these changes

@gumb0 gumb0 gumb0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chfast @gumb0 @axic