Separate _verify_single_precompiled_checksum() #1304
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pirapira
force-pushed
the
individual-verify
branch
from
595efcb to
73aaa34
Compare
pirapira
force-pushed
the
individual-verify
branch
2 times, most recently
from
2fa885b to
5d1d154
Compare
Codecov Report
@@ Coverage Diff @@
## master #1304 +/- ##
==========================================
+ Coverage 81.77% 81.94% +0.17%
==========================================
Files 21 21
Lines 1465 1468 +3
Branches 193 193
==========================================
+ Hits 1198 1203 +5
+ Misses 228 226 -2
Partials 39 39
Continue to review full report at Codecov.
|
pirapira
force-pushed
the
individual-verify
branch
from
b71bd66 to
2796726
Compare
pirapira
force-pushed
the
individual-verify
branch
from
2796726 to
84a6477
Compare
|
|
||
| def test_verify_single_precompiled_cyhecksum_on_nonexistent_contract_name() -> None: | ||
| with pytest.raises(ContractSourceManagerVerificationError, match="No checksum for"): | ||
| verify_single_precompiled_checksum_on_nonexistetnt_contract_name() |
There was a problem hiding this comment.
Suggested change
| verify_single_precompiled_checksum_on_nonexistetnt_contract_name() | |
| verify_single_precompiled_checksum_on_nonexistent_contract_name() |
| ) | ||
|
|
||
|
|
||
| def verify_single_precompiled_checksum_on_nonexistetnt_contract_name() -> None: |
There was a problem hiding this comment.
It appears, this functions exists only for testing behavior. So it seems a little odd to have it inside contract_source_manager.py -- can't you move it to, e.g. https://github.com/raiden-network/raiden-contracts/blob/master/raiden_contracts/tests/utils/contracts.py ?
There was a problem hiding this comment.
I'm trying to avoid using _xyz from other modules. @hackaugusto what do you suggest?
konradkonrad
approved these changes
Oct 30, 2019
pirapira
force-pushed
the
individual-verify
branch
from
16ba9e4 to
d9646fe
Compare
This commit separates a function that checks one entry of contracts.json. This eases the implementation of raiden-network#1257.
pirapira
force-pushed
the
individual-verify
branch
from
d9646fe to
e8f5dda
Compare
pirapira
force-pushed
the
individual-verify
branch
from
e8f5dda to
01fe451
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit separates a function that checks one entry of
contracts.json. This eases the implementation of #1257.
What this PR does
This is a pure refactoring that separates away a subroutine.
Before this PR, ContractSourceManager didn't have a function that checks a single entry of
contracrts.json. There was onlyverify_precompiled_checksums()that checks all entries ofcontracts.json. This PR separates_verify_single_precompiled_checksum()that checks a single entry ofcontracts.json.Why I'm making this PR
After this PR, I want to expose a feature to check a single
contracts.jsonentry. That will be useful for checking whether or not SecretRegistry's sources have changed.What's tricky about this PR (if any)
I had to take a moment to name the arguments of the new function.
Any reviewer can check these:
python_variableAnd before "merge" all checkboxes have to be checked. If you find redundant points, remove them.