Problem: python dependencies in integration tests are old #1325
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
yihuang
requested review from
mmsqe and
JayT106
and removed request for
a team
WalkthroughThe update focuses on enhancing the hashing operations in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
integration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.toml
Files selected for processing (1)
- nix/testenv.nix (1 hunks)
Additional comments: 1
nix/testenv.nix (1)
- 1-4: The upgrade to Python 3.11 is correctly implemented by setting the
pythonattribute topython311. This change aligns with the PR's objective to modernize the Python environment used in integration tests. Ensure that all dependencies and the project's code are compatible with Python 3.11 to avoid runtime issues.
mmsqe
approved these changes
Feb 20, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/test_gravity.py (1 hunks)
Additional comments: 1
integration_tests/test_gravity.py (1)
- 180-180: The change to remove unnecessary line breaks in the
send_transactioncall within the loop that funds all accounts in thetest_multiple_attestation_processingfunction simplifies the code and improves readability. This is a positive change as it makes the code more concise without affecting its functionality.
mmsqe
reviewed
Feb 20, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
integration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.toml
Files selected for processing (1)
- nix/testenv.nix (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- nix/testenv.nix
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
integration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.toml
Files selected for processing (2)
- integration_tests/eip712_utils.py (2 hunks)
- integration_tests/gravity_utils.py (2 hunks)
Additional comments: 2
integration_tests/gravity_utils.py (1)
- 3-3: The import of
keccakfrometh_hash.autoreplaces the previoussha3import. Ensure that all instances wheresha3was used are now correctly utilizingkeccakfor hashing operations. This change should not alter the expected outcomes of these operations, given thatkeccakis the underlying algorithm ofsha3. However, it's crucial to verify that thekeccakfunction is invoked correctly throughout the file.integration_tests/eip712_utils.py (1)
- 5-5: The import of
keccakfrometh_hash.autoreplaces the previoussha3import. This change is part of the effort to update dependencies to more current versions. Ensure that all instances wheresha3was previously used are now correctly utilizingkeccakfor hashing operations. This change should maintain the expected outcomes of these operations, given thatkeccakis the underlying algorithm ofsha3. However, it's crucial to verify that thekeccakfunction is invoked correctly throughout the file.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1325 +/- ##
===========================================
- Coverage 36.38% 15.90% -20.48%
===========================================
Files 118 80 -38
Lines 10755 6230 -4525
===========================================
- Hits 3913 991 -2922
+ Misses 6463 5160 -1303
+ Partials 379 79 -300 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (16)
- integration_tests/eip712_utils.py (3 hunks)
- integration_tests/protobuf/amino/amino_pb2.py (1 hunks)
- integration_tests/protobuf/cosmos/bank/v1beta1/bank_pb2.py (2 hunks)
- integration_tests/protobuf/cosmos/bank/v1beta1/tx_pb2.py (2 hunks)
- integration_tests/protobuf/cosmos/base/v1beta1/coin_pb2.py (1 hunks)
- integration_tests/protobuf/cosmos/crypto/multisig/v1beta1/multisig_pb2.py (2 hunks)
- integration_tests/protobuf/cosmos/crypto/secp256k1/keys_pb2.py (1 hunks)
- integration_tests/protobuf/cosmos/msg/v1/msg_pb2.py (1 hunks)
- integration_tests/protobuf/cosmos/tx/signing/v1beta1/signing_pb2.py (2 hunks)
- integration_tests/protobuf/cosmos/tx/v1beta1/tx_pb2.py (2 hunks)
- integration_tests/protobuf/cosmos_proto/cosmos_pb2.py (2 hunks)
- integration_tests/protobuf/ethermint/crypto/v1/ethsecp256k1/keys_pb2.py (2 hunks)
- integration_tests/protobuf/ethermint/evm/v1/query_pb2.py (1 hunks)
- integration_tests/protobuf/ethermint/types/v1/web3_pb2.py (2 hunks)
- integration_tests/protobuf/gogoproto/gogo_pb2.py (2 hunks)
- integration_tests/protobuf/google/protobuf/any_pb2.py (2 hunks)
Files not summarized due to errors (1)
- integration_tests/protobuf/ethermint/evm/v1/query_pb2.py: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- integration_tests/protobuf/cosmos/msg/v1/msg_pb2.py
Files skipped from review as they are similar to previous changes (1)
- integration_tests/eip712_utils.py
Additional comments: 36
integration_tests/protobuf/google/protobuf/any_pb2.py (3)
- 4-4: The Python version specified in the comment has been updated to reflect the new environment. This change is expected and aligns with the PR's objectives.
- 20-22: The use of
_builder.BuildMessageAndEnumDescriptorsand_builder.BuildTopDescriptorsAndMessagesfor building message and enum descriptors is standard practice in protobuf-generated Python files. These changes appear to be part of the routine update process and should not introduce issues.- 24-29: Adjustments to global variables and their options to accommodate the updated protobuf version are noted. It's crucial to ensure these changes are consistent across all protobuf-generated files to maintain uniformity and compatibility.
integration_tests/protobuf/ethermint/crypto/v1/ethsecp256k1/keys_pb2.py (3)
- 4-4: The Python version specified in the comment has been updated, which is consistent with the PR's objectives to update the Python environment.
- 20-22: The use of
_builderfunctions for building descriptors and messages is standard and expected in the context of protobuf-generated files. These changes are part of the routine update process.- 24-31: Adjustments to global variables and their options, similar to those seen in
any_pb2.py, are consistent with updates to the protobuf version. Ensuring these changes are uniform across all protobuf-generated files is crucial for compatibility.integration_tests/protobuf/amino/amino_pb2.py (3)
- 4-4: The Python version comment has been updated, aligning with the PR's goal to update the Python environment for integration tests.
- 20-22: Usage of
_builderfunctions for descriptor and message building is standard in protobuf-generated files. These changes are expected as part of the protobuf version update.- 24-26: Adjustments to global variables and their options are consistent with the protobuf version update, similar to changes observed in other protobuf-generated files.
integration_tests/protobuf/cosmos/crypto/multisig/v1beta1/multisig_pb2.py (3)
- 4-4: The Python version comment has been updated, which is consistent with the PR's objectives to update the Python environment.
- 20-22: The use of
_builderfunctions for building descriptors and messages is standard and expected in the context of protobuf-generated files. These changes are part of the routine update process.- 24-33: Adjustments to global variables and their options, similar to those seen in previous protobuf-generated files, are consistent with updates to the protobuf version. Ensuring these changes are uniform across all protobuf-generated files is crucial for compatibility.
integration_tests/protobuf/cosmos_proto/cosmos_pb2.py (3)
- 4-4: The Python version comment has been updated, aligning with the PR's goal to update the Python environment for integration tests.
- 20-22: Usage of
_builderfunctions for descriptor and message building is standard in protobuf-generated files. These changes are expected as part of the protobuf version update.- 24-31: Adjustments to global variables and their options are consistent with the protobuf version update, similar to changes observed in other protobuf-generated files.
integration_tests/protobuf/ethermint/types/v1/web3_pb2.py (3)
- 4-4: The Python version comment has been updated, which is consistent with the PR's objectives to update the Python environment.
- 20-22: The use of
_builderfunctions for building descriptors and messages is standard and expected in the context of protobuf-generated files. These changes are part of the routine update process.- 24-35: Adjustments to global variables and their options, similar to those seen in previous protobuf-generated files, are consistent with updates to the protobuf version. Ensuring these changes are uniform across all protobuf-generated files is crucial for compatibility.
integration_tests/protobuf/cosmos/tx/signing/v1beta1/signing_pb2.py (3)
- 4-4: The Python version comment has been updated, aligning with the PR's goal to update the Python environment for integration tests.
- 20-22: Usage of
_builderfunctions for descriptor and message building is standard in protobuf-generated files. These changes are expected as part of the protobuf version update.- 24-38: Adjustments to global variables and their options are consistent with the protobuf version update, similar to changes observed in other protobuf-generated files.
integration_tests/protobuf/cosmos/base/v1beta1/coin_pb2.py (3)
- 4-4: The Python version comment has been updated, which is consistent with the PR's objectives to update the Python environment.
- 22-24: The use of
_builderfunctions for building descriptors and messages is standard and expected in the context of protobuf-generated files. These changes are part of the routine update process.- 26-47: Adjustments to global variables and their options, similar to those seen in previous protobuf-generated files, are consistent with updates to the protobuf version. Ensuring these changes are uniform across all protobuf-generated files is crucial for compatibility.
integration_tests/protobuf/cosmos/bank/v1beta1/bank_pb2.py (5)
- 4-4: The update to Protobuf Python Version 4.25.2 is noted. This version update is crucial for ensuring compatibility with the latest protobuf features and improvements.
- 9-9: The use of
google.protobuf.internal.builderfor imports indicates a shift towards internal protobuf mechanisms for descriptor building. This change aligns with the updated protobuf version and its capabilities.- 18-19: The addition of imports from
cosmos.msg.v1andaminosuggests an expansion in the scope of functionalities covered by this protobuf file, potentially to accommodate new message types or cryptographic functions.- 22-69: Significant modifications have been made to the declarations of public entities like
Params,SendEnabled,Input,Output,Supply,DenomUnit, andMetadata, with alterations in field definitions and options. These changes are likely to impact how these entities are used throughout the codebase. Ensure that all references to these entities and their fields are updated accordingly to prevent runtime errors.- 26-69: The use of
_builder.BuildMessageAndEnumDescriptorsand subsequent adjustments to global variables and descriptor options reflect a more detailed control over protobuf descriptor behavior. This is particularly important for ensuring that the protobuf definitions are correctly interpreted and compiled by the protobuf runtime.integration_tests/protobuf/gogoproto/gogo_pb2.py (3)
- 4-4: The Protobuf Python version has been updated to 4.25.2. This update is crucial for compatibility with the latest features and bug fixes provided by the Protobuf library.
- 20-22: The use of
_builder.BuildMessageAndEnumDescriptorsand_builder.BuildTopDescriptorsAndMessagesfor building and registering descriptors is a standard approach in Protobuf-generated Python code. This ensures that the Python classes for Protobuf messages and enums are correctly set up in the module's global namespace.- 24-25: Setting
_optionsand_serialized_optionstoNoneand a byte string, respectively, for theDESCRIPTORis part of the Protobuf Python code generation process. This customization is typically used to specify options that affect the generated code or runtime behavior. However, directly manipulating these attributes is unusual in manually written code and should be carefully reviewed for correctness and necessity.Please verify the necessity and correctness of manually setting
_optionsand_serialized_optionsforDESCRIPTOR. This is typically handled by the Protobuf compiler and might not be needed or could lead to unexpected behavior if done incorrectly.integration_tests/protobuf/ethermint/evm/v1/query_pb2.py (4)
- 4-4: The Protobuf Python version has been updated to 4.25.2 in this file as well. This is a positive change, ensuring compatibility with the latest Protobuf features and improvements.
- 17-21: The addition of imports from various Protobuf files specific to the Ethermint project and Google APIs is necessary for the definitions within this file. These imports are standard practice in Protobuf-generated files and ensure that all necessary types are available for the Protobuf messages defined in this file.
- 27-29: Similar to the previous file, the use of
_builder.BuildMessageAndEnumDescriptorsand_builder.BuildTopDescriptorsAndMessagesis standard in Protobuf-generated Python code for setting up message and enum descriptors. This ensures the Python classes for Protobuf messages and enums are correctly registered.- 31-84: The manual setting of
_optionsand_serialized_optionstoNoneand specific byte strings for various descriptors is unusual in manually written code. This pattern is observed across multiple descriptors in this file, indicating a systematic approach to customizing Protobuf message options. While this might be necessary for certain advanced use cases, it's important to ensure that these customizations are indeed required and correctly implemented to avoid maintenance and compatibility issues.Please verify the necessity and correctness of manually setting
_optionsand_serialized_optionsfor various descriptors. Ensure that these customizations are required for the project's specific needs and that they are implemented correctly to avoid potential issues.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/test_ibc_rly.py (2 hunks)
Additional comments: 2
integration_tests/test_ibc_rly.py (2)
- 120-133: The use of
AttributeDictforpacketDataHexin therecv_packetfunction introduces a more structured and readable way to handle packet data. However, ensure that all consuming functions and processes are compatible with this change, asAttributeDictbehaves slightly differently from a standard dictionary, especially in terms of attribute access vs. key access.- 162-175: Similar to the
recv_packetfunction, thewrite_ackfunction's adoption ofAttributeDictforpacketDataHexenhances code readability and structure. It's crucial to verify that the changes are fully integrated and tested across the system to ensure no functionality is broken due to the differences in data access patterns betweenAttributeDictand standard dictionaries.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/test_ibc_rly.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/test_ibc_rly.py
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- integration_tests/eip712_utils.py (3 hunks)
- integration_tests/test_ibc_rly.py (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- integration_tests/eip712_utils.py
- integration_tests/test_ibc_rly.py
yihuang
commented
Feb 20, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- integration_tests/eip712_utils.py (5 hunks)
- integration_tests/gravity_utils.py (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- integration_tests/eip712_utils.py
- integration_tests/gravity_utils.py
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- integration_tests/test_filters.py (1 hunks)
- integration_tests/test_ica_precompile.py (2 hunks)
- integration_tests/test_pruned_node.py (1 hunks)
Additional comments: 5
integration_tests/test_filters.py (2)
- 37-37: The method call
create_filtercorrectly follows Python's naming conventions and the Web3.py library's standards for method names, improving readability and consistency.- 43-43: The use of
process_receiptmethod aligns with Python's naming conventions and the Web3.py library's standards. This change enhances code readability and consistency.integration_tests/test_pruned_node.py (1)
- 100-100: Converting the "input" field to a
HexBytesobject enhances type safety and compatibility with the Web3.py library. This change ensures that hexadecimal values are correctly represented and handled within the code.integration_tests/test_ica_precompile.py (2)
- 124-124: Updating the method call from
event.getLogs()toevent.get_logs()aligns with Python's naming conventions and enhances code readability and consistency.- 198-198: The change to
event.get_logs()fromevent.getLogs()is consistent with Python's naming conventions, improving code readability and consistency.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/eip712_utils.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/eip712_utils.py
Solution: - update python to 3.11 - update dependencies fix lint add eth-bloom add eth-bloom use eth_hash inplace of pysha3 gen pb2.py with protoc fix format fix format fix test Revert "gen pb2.py with protoc" This reverts commit cbb83ca. fix dup proto fix sha fix web3.py fix test cleanup amount_dict
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
integration_tests/poetry.lockis excluded by:!**/*.lockintegration_tests/pyproject.tomlis excluded by:!**/*.toml
Files selected for processing (8)
- integration_tests/eip712_utils.py (5 hunks)
- integration_tests/gravity_utils.py (2 hunks)
- integration_tests/test_filters.py (1 hunks)
- integration_tests/test_gravity.py (1 hunks)
- integration_tests/test_ibc_rly.py (4 hunks)
- integration_tests/test_ica_precompile.py (2 hunks)
- integration_tests/test_pruned_node.py (1 hunks)
- nix/testenv.nix (1 hunks)
Files skipped from review as they are similar to previous changes (8)
- integration_tests/eip712_utils.py
- integration_tests/gravity_utils.py
- integration_tests/test_filters.py
- integration_tests/test_gravity.py
- integration_tests/test_ibc_rly.py
- integration_tests/test_ica_precompile.py
- integration_tests/test_pruned_node.py
- nix/testenv.nix
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- integration_tests/eip712_utils.py (5 hunks)
- integration_tests/gravity_utils.py (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- integration_tests/eip712_utils.py
- integration_tests/gravity_utils.py
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/eip712_utils.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/eip712_utils.py
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/eip712_utils.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/eip712_utils.py
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/eip712_utils.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/eip712_utils.py
Merged
via the queue into
crypto-org-chain:main
with commit Feb 21, 2024
e16b913
41 of 42 checks passed
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.
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit