Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: add test coverage for RPC downloader and importer offline e2e #2029

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

gabriel-aranha-cw
Copy link
Contributor

@gabriel-aranha-cw gabriel-aranha-cw commented Feb 26, 2025

User description

Add build step to e2e test recipes for RPC downloader and importer offline Extend stratus-test-coverage recipe to include additional test scenarios Clean up temporary RocksDB database after test coverage run


PR Type

Enhancement, Tests


Description

  • Add build step to RPC downloader and importer offline e2e tests

  • Extend stratus-test-coverage recipe with additional test scenarios

  • Clean up temporary RocksDB database after test coverage run


Changes walkthrough 📝

Relevant files
Enhancement
justfile
Improve e2e test recipes and coverage                                       

justfile

  • Added build step to e2e-rpc-downloader and e2e-importer-offline
    recipes
  • Extended stratus-test-coverage recipe with new test scenarios
  • Added cleanup for temporary RocksDB database
  • +10/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Add build step to e2e test recipes for RPC downloader and importer offline
    Extend stratus-test-coverage recipe to include additional test scenarios
    Clean up temporary RocksDB database after test coverage run
    Copy link

    github-actions bot commented Feb 26, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 8d05ae0)

    Here are some key observations to aid the review process:

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

    Copy link

    github-actions bot commented Feb 26, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 8d05ae0
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add error handling for removal

    Add error handling for the 'rm' command to prevent script failure if the directory
    doesn't exist.

    justfile [672]

    --rm -r data/importer-offline-database-rocksdb
    +rm -rf data/importer-offline-database-rocksdb || true
    Suggestion importance[1-10]: 6

    __

    Why: Adding error handling for the 'rm' command prevents potential script failures, which is a useful improvement for script robustness and reliability.

    Low
    Use variable for Stratus address

    Consider using a variable or constant for the Stratus address (0.0.0.0:3000) to
    maintain consistency and ease future changes.

    justfile [442-443]

    -just run -a 0.0.0.0:3000 > e2e_logs/e2e-rpc-downloader-stratus.log &
    +STRATUS_ADDRESS="0.0.0.0:3000"
    +just run -a $STRATUS_ADDRESS > e2e_logs/e2e-rpc-downloader-stratus.log &
     just _wait_for_stratus
    Suggestion importance[1-10]: 5

    __

    Why: Using a variable for the Stratus address improves maintainability and consistency, but it's a minor improvement that doesn't address a critical issue.

    Low

    Previous suggestions

    Suggestions up to commit 8d05ae0
    CategorySuggestion                                                                                                                                    Impact
    General
    Add error handling for removal

    Add error handling for the 'rm' command to prevent script failure if the directory
    doesn't exist.

    justfile [672]

    --rm -r data/importer-offline-database-rocksdb
    +rm -rf data/importer-offline-database-rocksdb || true
    Suggestion importance[1-10]: 6

    __

    Why: Adding error handling for the 'rm' command prevents potential script failures, which is a useful improvement for script robustness and reliability.

    Low
    Use variable for Stratus address

    Consider using a variable or constant for the Stratus address (0.0.0.0:3000) to
    maintain consistency and ease future changes.

    justfile [442-443]

    -just run -a 0.0.0.0:3000 > e2e_logs/e2e-rpc-downloader-stratus.log &
    +STRATUS_ADDRESS="0.0.0.0:3000"
    +just run -a $STRATUS_ADDRESS > e2e_logs/e2e-rpc-downloader-stratus.log &
     just _wait_for_stratus
    Suggestion importance[1-10]: 5

    __

    Why: Using a variable for the Stratus address improves maintainability and consistency, but it's a minor enhancement that doesn't address a critical issue.

    Low

    @gabriel-aranha-cw gabriel-aranha-cw marked this pull request as ready for review February 26, 2025 17:06
    @gabriel-aranha-cw gabriel-aranha-cw enabled auto-merge (squash) February 26, 2025 17:06
    Copy link

    Persistent review updated to latest commit 8d05ae0

    @gabriel-aranha-cw gabriel-aranha-cw merged commit 0836314 into main Feb 26, 2025
    42 checks passed
    @gabriel-aranha-cw gabriel-aranha-cw deleted the fix-new-e2e-tests-and-coverage branch February 26, 2025 17:49
    # for free to join this conversation on GitHub. Already have an account? # to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants