ci: 2598 artillery based stress testing is inaccurate due to misconfiguration

[morde08]

Description

• Update Artillery performance testing config to run valid EVM tests

How Has This Been Tested?

• Tested CCTX in localnet
• Tested in development environment
• Go unit tests
• Go integration tests
• Tested via GitHub Actions

Summary by CodeRabbit

• New Features

  • Enhanced performance testing with new plugins for improved validation.
  • Support for multiple acceptable status codes (200 and 201) in performance tests.
  • Improved response validation to check for the absence of errors.

• Bug Fixes

  • Removed outdated failure threshold configuration for clearer testing criteria.

• Documentation

  • Updated configuration and scenarios for better clarity and structure in performance testing.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

📝 Walkthrough

Walkthrough

The pull request introduces modifications to the performance testing configuration in the art.yaml file. Key updates include the addition of two plugins, expect and ensure, and the removal of the previous failure threshold in the ensure section. The ensure section now accepts multiple status codes, while the assert section has been replaced with an expect section that checks for the absence of errors in responses. Additionally, various method parameters have been updated to include specific values rather than placeholders.

Changes

File Path

Change Summary

.github/actions/performance-tests/art.yaml

- Added expect and ensure plugins to configuration defaults. - Removed failure threshold from ensure section. - Updated ensure section to use a list format for status codes. - Replaced assert section with expect to check for absence of response.error. - Modified parameters in various methods to include specific addresses and block hashes. - Simplified as field in capture section by removing quotes around response.

Suggested labels

no-changelog

Suggested reviewers

• fbac
• swift1337
• kingpinXD
• ws4charlie
• lumtis
• brewmaster012

---

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?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (14)

.github/actions/performance-tests/art.yaml (14)

12-13: Enhance plugin configuration for improved clarity.

The addition of the expect and ensure plugins is a positive change that will enhance the test validation capabilities. However, to improve clarity and maintainability, consider explicitly defining the configuration options for these plugins, even if they are currently using default settings.

Consider updating the plugin configuration as follows:

plugins:
  expect:
    outputFormat: "json" # or any other relevant options
  ensure:
    thresholds: [] # or any other relevant options

This approach will make it easier for future maintainers to understand and modify the configuration as needed.

---

41-47: Approve changes and suggest error message capture.

The modifications to the web3_clientVersion scenario enhance its readability and error-checking capabilities. The use of a list for status codes in the ensure section provides flexibility, and the addition of the expect section improves error detection.

To further enhance error handling, consider capturing the error message when present:

expect:
  - notHasProperty: "{{ response.error }}"
  - statusCode:
      - 200
      - 201
capture:
  - json: "$.error.message"
    as: errorMessage
    when: "{{ response.error }}"

This approach will allow for more detailed error reporting in case of test failures.

---

99-108: Approve changes and suggest type-aware comparison.

The modifications to the net_listening scenario are consistent with previous improvements and include an additional expectation to validate the result.

To enhance the robustness of the test, consider using a type-aware comparison:

expect:
  - notHasProperty: "{{ response.error }}"
  - equals:
      - true
      - "{{ JSON.parse(response.result) }}"

This approach ensures that the comparison is performed against a boolean true rather than the string "true", which is more accurate for JSON-RPC responses.

---

177-187: Approve changes and suggest parameterization for improved flexibility.

The addition of a specific block hash in the eth_getTransactionByBlockHashAndIndex scenario makes the test more concrete. The changes to the ensure and expect sections are consistent with previous improvements.

To enhance the flexibility of the test, consider parameterizing the block hash:

config:
  variables:
    testBlockHash: "0xbd8bd16e5d4375ed4bab633c4fb8aca58fdf7ed6c340853b20cfa91ed9a3b9e1"

scenarios:
  - name: eth_getTransactionByBlockHashAndIndex
    flow:
      - post:
          # ...
          params:
            - "{{ testBlockHash }}"
            - "0x0"

This approach allows for easier updates and potential dynamic generation of test data.

---

261-273: Approve changes and suggest parameterization for improved flexibility.

The addition of specific filter parameters in the eth_newFilter scenario, including a concrete address, enhances the test's specificity. The changes to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and reusability, consider parameterizing the filter options:

config:
  variables:
    testAddress: "0x81eF14691E9ea2f711cf56a9f0889c49C5Fe995a"

scenarios:
  - name: eth_newFilter
    flow:
      - post:
          # ...
          params:
            - fromBlock: "{{ fromBlock | default('latest') }}"
              toBlock: "{{ toBlock | default('latest') }}"
              address: "{{ testAddress }}"
              topics: "{{ topics | default([]) }}"

This approach allows for easier updates and potential dynamic generation of test data, enhancing the scenario's adaptability to different test cases.

---

284-293: Approve changes and suggest dynamic filter ID generation.

The addition of a specific filter ID in the eth_getFilterChanges scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and ensure it uses a valid filter ID, consider dynamically generating the filter ID:

scenarios:
  - name: eth_newFilter
    flow:
      - post:
          # ... (previous eth_newFilter scenario)
      - capture:
          - json: "$.result"
            as: filterId

  - name: eth_getFilterChanges
    flow:
      - post:
          # ...
          params:
            - "{{ filterId }}"

This approach ensures that the eth_getFilterChanges call uses a filter ID created by a preceding eth_newFilter call, making the test more robust and realistic.

---

343-353: Approve changes and suggest dynamic block hash retrieval.

The addition of a specific block hash in the eth_getBlockByHash scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and ensure it uses a valid, recent block hash, consider dynamically retrieving a block hash:

scenarios:
  - name: eth_getBlockByNumber
    flow:
      - post:
          # ... (eth_getBlockByNumber scenario)
      - capture:
          - json: "$.result.hash"
            as: recentBlockHash

  - name: eth_getBlockByHash
    flow:
      - post:
          # ...
          params:
            - "{{ recentBlockHash }}"
            - true

This approach ensures that the eth_getBlockByHash call uses a recent, valid block hash, making the test more robust and adaptable to different network states.

---

385-394: Approve changes and suggest reuse of dynamic block hash.

The addition of a specific block hash in the eth_getBlockTransactionCountByHash scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and maintain consistency with other scenarios, consider reusing the dynamically retrieved block hash suggested in the eth_getBlockByHash scenario:

scenarios:
  - name: eth_getBlockTransactionCountByHash
    flow:
      - post:
          # ...
          params:
            - "{{ recentBlockHash }}"

This approach ensures that the eth_getBlockTransactionCountByHash call uses the same recent, valid block hash as other scenarios, maintaining consistency across the test suite and adapting to different network states.

---

425-435: Approve changes and suggest parameterization of contract address.

The addition of a specific contract address in the eth_getCode scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and reusability, consider parameterizing the contract address:

config:
  variables:
    testContractAddress: "0x0cbe0dF132a6c6B4a2974Fa1b7Fb953CF0Cc798a"

scenarios:
  - name: eth_getCode
    flow:
      - post:
          # ...
          params:
            - "{{ testContractAddress }}"
            - "latest"

This approach allows for easier updates and potential dynamic generation of test data, enhancing the scenario's adaptability to different test cases or contract deployments.

---

446-455: Approve changes and suggest dynamic transaction hash retrieval.

The addition of a specific transaction hash in the eth_getTransactionByHash scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and ensure it uses a valid, recent transaction hash, consider dynamically retrieving a transaction hash:

scenarios:
  - name: eth_getBlockByNumber
    flow:
      - post:
          # ... (eth_getBlockByNumber scenario)
      - capture:
          - json: "$.result.transactions[0]"
            as: recentTransactionHash

  - name: eth_getTransactionByHash
    flow:
      - post:
          # ...
          params:
            - "{{ recentTransactionHash }}"

This approach ensures that the eth_getTransactionByHash call uses a recent, valid transaction hash, making the test more robust and adaptable to different network states.

---

466-475: Approve changes and suggest reuse of dynamic transaction hash.

The addition of a specific transaction hash in the eth_getTransactionReceipt scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and maintain consistency with other scenarios, consider reusing the dynamically retrieved transaction hash suggested in the eth_getTransactionByHash scenario:

scenarios:
  - name: eth_getTransactionReceipt
    flow:
      - post:
          # ...
          params:
            - "{{ recentTransactionHash }}"

This approach ensures that the eth_getTransactionReceipt call uses the same recent, valid transaction hash as other scenarios, maintaining consistency across the test suite and adapting to different network states.

---

486-495: Approve changes and suggest parameterization of contract address.

The addition of a specific contract address in the eth_getLogs scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and reusability, consider parameterizing the contract address and reusing the variable suggested in the eth_getCode scenario:

scenarios:
  - name: eth_getLogs
    flow:
      - post:
          # ...
          params:
            - address: "{{ testContractAddress }}"
              fromBlock: "{{ fromBlock | default('latest') }}"
              toBlock: "{{ toBlock | default('latest') }}"

This approach allows for easier updates and potential dynamic generation of test data, enhancing the scenario's adaptability to different test cases or contract deployments while maintaining consistency with other scenarios.

---

506-516: Approve changes and suggest parameterization of account address.

The addition of a specific account address in the eth_getBalance scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and reusability, consider parameterizing the account address:

config:
  variables:
    testAccountAddress: "0x735b14BB79463307AAcBED86DAf3322B1e6226aB"

scenarios:
  - name: eth_getBalance
    flow:
      - post:
          # ...
          params:
            - "{{ testAccountAddress }}"
            - "latest"

This approach allows for easier updates and potential dynamic generation of test data, enhancing the scenario's adaptability to different test cases or account states.

---

527-537: Approve changes and suggest parameterization of addresses.

The addition of specific 'from' and 'to' addresses in the eth_estimateGas scenario enhances test specificity. The modifications to the ensure and expect sections maintain consistency with previous improvements.

To improve the test's flexibility and reusability, consider parameterizing the addresses:

config:
  variables:
    testFromAddress: "0x239e96c8f17C85c30100AC26F635Ea15f23E9c67"
    testToAddress: "0x0cbe0dF132a6c6B4a2974Fa1b7Fb953CF0Cc798a"

scenarios:
  - name: eth_estimateGas
    flow:
      - post:
          # ...
          params:
            - from: "{{ testFromAddress }}"
              to: "{{ testToAddress }}"
              value: "0x0" # Consider adding a parameterized value

This approach allows for easier updates and potential dynamic generation of test data, enhancing the scenario's adaptability to different test cases or account states.

🧰 Tools

🪛 yamllint

[error] 537-537: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between f9cbcfc and cdb801a.

📒 Files selected for processing (1)

• .github/actions/performance-tests/art.yaml (27 hunks)

🧰 Additional context used

🪛 yamllint

.github/actions/performance-tests/art.yaml

[warning] 67-67: wrong indentation: expected 12 but found 10

(indentation)

---

[error] 537-537: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (1)

.github/actions/performance-tests/art.yaml (1)

61-67: Consistent improvements applied to web3_sha3 scenario.

The changes to the web3_sha3 scenario are consistent with the improvements made to the web3_clientVersion scenario, enhancing readability and error-checking capabilities.

Please refer to the suggestion made for the web3_clientVersion scenario regarding error message capture, as it applies here as well.

🧰 Tools

🪛 yamllint

[warning] 67-67: wrong indentation: expected 12 but found 10

(indentation)

[kingpinXD]

Can you share a link to see the before and after ?