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: contract availability after deployment #1572

Merged
merged 2 commits into from
Jul 30, 2024

Conversation

carneiro-cw
Copy link
Contributor

@carneiro-cw carneiro-cw commented Jul 30, 2024

PR Type

Tests


Description

  • Added tests to check that contract code is available in the block it was deployed.
  • Wrapped eth_getCode related tests in a describe block for better organization.
  • Included sendReset calls before eth_blockNumber tests to ensure a clean state.

Changes walkthrough 📝

Relevant files
Tests
e2e-json-rpc.test.ts
Add tests for contract code availability and block number reset

e2e/test/automine/e2e-json-rpc.test.ts

  • Added a test to check contract code availability in the block it was
    deployed.
  • Wrapped eth_getCode tests in a describe block.
  • Added sendReset before eth_blockNumber test.
  • +8/-1     
    e2e-json-rpc.test.ts
    Add tests for contract code availability and block number reset

    e2e/test/external/e2e-json-rpc.test.ts

  • Added a test to check contract code availability in the block it was
    deployed.
  • Wrapped eth_getCode tests in a describe block.
  • Added sendReset before eth_blockNumber test.
  • +11/-1   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Duplication
    The test for checking contract code availability in the block it was deployed is duplicated in both e2e/test/automine/e2e-json-rpc.test.ts and e2e/test/external/e2e-json-rpc.test.ts. Consider refactoring to avoid duplication.

    Code Duplication
    The test for checking contract code availability in the block it was deployed is duplicated in both e2e/test/automine/e2e-json-rpc.test.ts and e2e/test/external/e2e-json-rpc.test.ts. Consider refactoring to avoid duplication.

    Copy link

    github-actions bot commented Jul 30, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure the block containing the contract deployment is mined before checking the code

    Add a call to await sendEvmMine(); after deploying the contract to ensure that the
    block containing the contract deployment is mined before checking the code.

    e2e/test/automine/e2e-json-rpc.test.ts [88-89]

     const contract = await deployTestContractBalances();
    +await sendEvmMine();
     (await sendExpect("eth_getCode", [contract.target, "latest"])).not.eq("0x");
     
    Suggestion importance[1-10]: 10

    Why: This suggestion ensures that the block containing the contract deployment is mined before checking the code, which is crucial for the accuracy of the test.

    10
    Possible issue
    Add a check to ensure the contract is deployed successfully before performing further tests

    Add a check to ensure that the deployTestContractBalances function successfully
    deploys the contract before proceeding with the eth_getCode checks. This will help
    catch any deployment failures early.

    e2e/test/automine/e2e-json-rpc.test.ts [88-89]

     const contract = await deployTestContractBalances();
    +if (!contract) {
    +    throw new Error("Contract deployment failed");
    +}
     (await sendExpect("eth_getCode", [contract.target, "latest"])).not.eq("0x");
     
    Suggestion importance[1-10]: 9

    Why: This suggestion adds a necessary check to ensure the contract deployment was successful, which can help catch deployment failures early and prevent false positives in the test results.

    9
    Maintainability
    Remove redundant await sendEvmMine(); call after deploying the contract

    Remove the redundant await sendEvmMine(); call after deploying the contract since it
    is already present in the next line.

    e2e/test/external/e2e-json-rpc.test.ts [96-97]

     const contract = await deployTestContractBalances();
     await sendEvmMine();
    -await sendEvmMine();
     
    Suggestion importance[1-10]: 8

    Why: Removing the redundant await sendEvmMine(); call improves code maintainability and readability by eliminating unnecessary duplication.

    8

    @carneiro-cw carneiro-cw merged commit fb66633 into main Jul 30, 2024
    32 checks passed
    @carneiro-cw carneiro-cw deleted the test_contract_deployment branch July 30, 2024 20:50
    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.

    1 participant