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

E2E Test #64

Merged
merged 2 commits into from
Oct 1, 2024
Merged

E2E Test #64

merged 2 commits into from
Oct 1, 2024

Conversation

bh2smith
Copy link
Collaborator

@bh2smith bh2smith commented Oct 1, 2024

User description

Testing the bundler requests.


PR Type

Tests, Enhancement


Description

  • Added a new end-to-end test for NearSafe requests in tests/e2e.spec.ts.
  • Updated import paths in multiple unit test files to reflect new directory structure.
  • Added yarn test unit command to the pull request workflow to ensure unit tests are run.

Changes walkthrough 📝

Relevant files
Tests
e2e.spec.ts
Add end-to-end test for NearSafe requests                               

tests/e2e.spec.ts

  • Added new end-to-end test for NearSafe requests.
  • Configured dotenv for environment variables.
  • Tested buildTransaction method with different scenarios.
  • +43/-0   
    Enhancement
    ethers-safe.ts
    Update import paths in ethers-safe unit test                         

    tests/unit/ethers-safe.ts

    • Updated import paths to reflect new directory structure.
    +2/-2     
    bundler.spec.ts
    Update import paths in bundler unit test                                 

    tests/unit/lib/bundler.spec.ts

    • Updated import paths to reflect new directory structure.
    +1/-1     
    multisend.spec.ts
    Update import paths in multisend unit test                             

    tests/unit/lib/multisend.spec.ts

    • Updated import paths to reflect new directory structure.
    +1/-1     
    safe-message.spec.ts
    Update import paths in safe-message unit test                       

    tests/unit/lib/safe-message.spec.ts

    • Updated import paths to reflect new directory structure.
    +1/-1     
    safe.spec.ts
    Update import paths in safe unit test                                       

    tests/unit/lib/safe.spec.ts

    • Updated import paths to reflect new directory structure.
    +1/-1     
    utils.spec.ts
    Update import paths in utils unit test                                     

    tests/unit/utils.spec.ts

    • Updated import paths to reflect new directory structure.
    +2/-2     
    Configuration changes
    pull-request.yaml
    Add unit tests to pull request workflow                                   

    .github/workflows/pull-request.yaml

    • Added yarn test unit command to the pull request workflow.
    +1/-0     

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

    @bh2smith bh2smith merged commit 374b29a into main Oct 1, 2024
    1 check passed
    @bh2smith bh2smith deleted the e2e-test branch October 1, 2024 11:38
    @mintbase-codium-pr-agent
    Copy link

    PR Reviewer Guide 🔍

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

    @mintbase-codium-pr-agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling or default value for environment variables to prevent runtime errors

    Consider handling the case where process.env.PIMLICO_KEY is undefined to prevent
    runtime errors. You can provide a default value or throw a custom error if the key
    is not found.

    tests/e2e.spec.ts [11]

    -pimlicoKey: process.env.PIMLICO_KEY!
    +pimlicoKey: process.env.PIMLICO_KEY || 'default-key'
     
    Suggestion importance[1-10]: 9

    Why: Handling undefined environment variables is crucial to prevent runtime errors, especially in a testing environment. This suggestion improves robustness.

    9
    Best practice
    Define repeated literal values as constants to improve maintainability and avoid magic numbers

    Since chainId is repeated with the same value in multiple calls, consider defining
    it as a constant at the beginning of your test suite to avoid magic numbers and make
    it easier to update in the future.

    tests/e2e.spec.ts [19]

    -chainId: 11155111,
    +const TEST_CHAIN_ID = 11155111;
    +chainId: TEST_CHAIN_ID,
     
    Suggestion importance[1-10]: 8

    Why: Defining repeated literal values as constants is a best practice that enhances maintainability and readability, making future updates easier.

    8
    Maintainability
    Extract repeated transaction objects into a separate variable for better readability and maintainability

    To improve code readability and maintainability, consider extracting the transaction
    object into a separate variable or function, especially since it is reused in
    multiple test cases.

    tests/e2e.spec.ts [20-24]

    -transactions: [
    -  {
    -    to: adapter.mpcAddress,
    -    ...irrelevantData,
    -  },
    -]
    +const transaction = {
    +  to: adapter.mpcAddress,
    +  ...irrelevantData,
    +};
    +transactions: [transaction]
     
    Suggestion importance[1-10]: 7

    Why: Extracting repeated code into a variable improves readability and maintainability, but the impact is relatively minor since it only affects a small part of the test.

    7
    Enhancement
    Use descriptive test names to clearly state what the test is verifying

    Consider using a more descriptive test case name than "buildTransaction" to reflect
    what is specifically being tested, such as
    "buildTransactionShouldNotThrowForValidInput".

    tests/e2e.spec.ts [7]

    -it("buildTransaction", async () => {
    +it("buildTransactionShouldNotThrowForValidInput", async () => {
     
    Suggestion importance[1-10]: 6

    Why: Using descriptive test names improves clarity and understanding of the test's purpose, but it is a minor enhancement in terms of code functionality.

    6

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant