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

add unit tests for account.ts #176

Merged
merged 2 commits into from
Mar 29, 2025
Merged

Conversation

ahmxdiqbal
Copy link
Member

@ahmxdiqbal ahmxdiqbal commented Mar 26, 2025

PR Type

Tests


Description

  • Add comprehensive unit tests for accounts module.

  • Mock dependencies for isolated testing.

  • Validate database operations and error handling.

  • Ensure logging behavior under different configurations.


Changes walkthrough 📝

Relevant files
Tests
accounts.test.ts
Add unit tests for account operations with mocks                 

test/unit/src/dbstore/accounts.test.ts

  • Added unit tests for account operations.
  • Mocked database and utility dependencies.
  • Tested logging for both success and error scenarios.
  • Verified behavior with different configuration settings.
  • +796/-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.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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


    // Verify
    expect(db.get).toHaveBeenCalledTimes(1)
    expect(result).toBeUndefined()

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Ensure that the test case for a non-existent account returns null instead of undefined to maintain consistency with the function's expected behavior. [possible issue, importance: 8]

    Suggested change
    expect(result).toBeUndefined()
    expect(result).toBeNull()

    // Verify
    expect(db.all).not.toHaveBeenCalled()
    expect(Logger.mainLogger.error).toHaveBeenCalledWith('queryAccounts - Invalid skip or limit value')
    expect(result).toEqual([])

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Verify that the test case for invalid skip or limit values returns null instead of an empty array to align with the function's expected behavior. [possible issue, importance: 8]

    Suggested change
    expect(result).toEqual([])
    expect(result).toBeNull()

    // Verify
    expect(db.all).toHaveBeenCalledTimes(1)
    expect(Logger.mainLogger.error).toHaveBeenCalledWith(error)
    expect(result).toEqual([])

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Ensure that the test case for a failed query returns null instead of an empty array to match the function's expected behavior. [possible issue, importance: 8]

    Suggested change
    expect(result).toEqual([])
    expect(result).toBeNull()

    @mhanson-github mhanson-github merged commit 65822c6 into mainnet-launch Mar 29, 2025
    5 checks passed
    @mhanson-github mhanson-github deleted the unit-test/accounts branch March 29, 2025 05:20
    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.

    3 participants