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

test: tests for functionality fixed in #2471 #2568

Merged
merged 19 commits into from
Jan 1, 2022

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Dec 15, 2021

What I did

PR for various tests associated with #2471

How I did it

Iterate through the list of functionalities fixed.

How to verify it

Run the test suite.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@tserg tserg changed the title chore: clamp uint8 returndata chore: tests for functionality fixed in #2471 Dec 15, 2021
@tserg
Copy link
Collaborator Author

tserg commented Dec 15, 2021

Hi, I am working on the tests for #2454 and have added the test cases for uint8. Seeking some feedback on whether I am in the right direction before I proceed further.

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021

Codecov Report

Merging #2568 (13a95f5) into master (c843b68) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2568      +/-   ##
==========================================
+ Coverage   85.66%   85.69%   +0.02%     
==========================================
  Files          90       90              
  Lines        9036     9036              
  Branches     2328     2328              
==========================================
+ Hits         7741     7743       +2     
+ Misses        814      813       -1     
+ Partials      481      480       -1     
Impacted Files Coverage Δ
vyper/lll/compile_lll.py 94.72% <0.00%> (ø)
vyper/old_codegen/abi.py 71.14% <0.00%> (+0.65%) ⬆️

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 c843b68...13a95f5. Read the comment docs.

@tserg tserg marked this pull request as draft December 21, 2021 02:32
@tserg
Copy link
Collaborator Author

tserg commented Dec 22, 2021

I have added a test case for #2455. It seems that the nonreentrant decorator is invoked even on a callback from an unprotected function without any nonreentrant decorator (see unprotected_function in test_nonreentrant_decorator_for_default under tests/parser/features/decorators/test_nonreentrant.py). I am not sure if this is intended behaviour or not.

@tserg
Copy link
Collaborator Author

tserg commented Dec 22, 2021

I have added a test case for #2455. It seems that the nonreentrant decorator is invoked even on a callback from an unprotected function without any nonreentrant decorator (see unprotected_function in test_nonreentrant_decorator_for_default under tests/parser/features/decorators/test_nonreentrant.py). I am not sure if this is intended behaviour or not.

Update: This behaviour is not intended. The callback transaction reverted due to out of gas. Changing to raw_call() resolved the issue. The test has been updated and the behaviour is now working as intended. Further, the documentation has been updated to recommend against using the @nonreentrant(<key>) decorator on the __default__ function. Thanks to @fubuloubu and @charles-cooper for guidance.

docs/control-structures.rst Outdated Show resolved Hide resolved
@tserg tserg marked this pull request as ready for review December 28, 2021 04:35
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

Left a couple questions, for the most part looks great! Thank you

tests/functional/codegen/test_abi_encode.py Outdated Show resolved Hide resolved
tests/functional/codegen/test_abi_encode.py Show resolved Hide resolved
tests/parser/features/decorators/test_nonreentrant.py Outdated Show resolved Hide resolved


@pytest.mark.parametrize("type", ["uint256", "int256"])
def test_address_too_long(get_contract, assert_tx_failed, type):
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is some way to parametrize these tests across types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean something like this: def test_{type}_too_long()?

@fubuloubu fubuloubu changed the title chore: tests for functionality fixed in #2471 test: tests for functionality fixed in #2471 Dec 31, 2021
@fubuloubu fubuloubu merged commit fda08e7 into vyperlang:master Jan 1, 2022
@tserg tserg deleted the chore/2471 branch January 13, 2022 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants