Skip to content

Conversation

kriptonian1
Copy link
Contributor

@kriptonian1 kriptonian1 commented Jun 22, 2025

User description

Description

Write unit test for custom hooks

Dependencies

  • ts-jest
  • jest
  • jest-environment-jsdom
  • @testing-library/react
  • @testing-library/react-hooks

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Tests


Description

  • Add comprehensive unit tests for useIsAppleDevice hook

  • Configure Jest testing environment with TypeScript support

  • Set up testing dependencies and module path mapping

  • Update package scripts for platform testing


Changes walkthrough 📝

Relevant files
Tests
use-is-apple-device.test.ts
Add comprehensive useIsAppleDevice hook tests                       

apps/platform/tests/hooks/use-is-apple-device.test.ts

  • Create comprehensive test suite for useIsAppleDevice hook
  • Test Apple device detection across multiple browsers and platforms
  • Include helper function for User Agent string testing
  • Cover Mac, iPhone, iPad, Windows, Linux, and Android scenarios
  • +169/-0 
    Configuration changes
    jest.config.ts
    Configure Jest testing environment                                             

    apps/platform/jest.config.ts

  • Configure Jest with TypeScript and jsdom environment
  • Set up module name mapping for @/ and @public/ paths
  • Define test file patterns and transform settings
  • +17/-7   
    package.json
    Update platform test command                                                         

    package.json

    • Update platform test command with --no-deps flag
    +1/-1     
    tsconfig.json
    Include test files in TypeScript config                                   

    apps/platform/tsconfig.json

  • Include test files and Jest config in TypeScript compilation
  • Add __tests__/**/*.ts and jest.config.ts to include array
  • +3/-1     
    Dependencies
    package.json
    Add testing dependencies and scripts                                         

    apps/platform/package.json

  • Add testing dependencies: jest, ts-jest, @testing-library packages
  • Include test script command
  • Set module type to ESM
  • +10/-1   
    Additional files
    pnpm-lock.yaml [link]   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Edge Test Case Issue

    The test for Edge on Mac uses a string replacement on Safari user agent instead of defining a proper Edge on Mac user agent string in the osAgentPairs object.

      testUserAgent(osAgentPairs.mac.safari.replace('Safari', 'Edg'), true)
    })
    Test Cleanup

    The testUserAgent function modifies navigator.userAgent but only restores it with Object.defineProperty which may not fully restore the original configuration. Consider using jest.spyOn for mocking instead.

    function testUserAgent(userAgent: string, expectedResult: boolean) {
      const originalUserAgent = window.navigator.userAgent
      Object.defineProperties(window.navigator, {
        userAgent: {
          value: userAgent,
          writable: true
        }
      })
    
      const { result } = renderHook(() => useIsAppleDevice())
    
      expect(result.current.isApple).toBe(expectedResult)
    
      // Restore the original User Agent
      Object.defineProperty(window.navigator, 'userAgent', {
        value: originalUserAgent
      })
    }

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Jun 22, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix property restoration
    Suggestion Impact:The commit addressed the same issue as the suggestion but used a different approach. Instead of manipulating the property descriptor, the commit used Jest's spyOn to mock the userAgent getter and properly restore it after the test.

    code diff:

    -  const originalUserAgent = window.navigator.userAgent
    -  Object.defineProperties(window.navigator, {
    -    userAgent: {
    -      value: userAgent,
    -      writable: true
    -    }
    -  })
    +  const spy = jest.spyOn(window.navigator, 'userAgent', 'get')
    +  spy.mockReturnValue(userAgent)
     
       const { result } = renderHook(() => useIsAppleDevice())
     
       expect(result.current.isApple).toBe(expectedResult)
     
    -  // Restore the original User Agent
    -  Object.defineProperty(window.navigator, 'userAgent', {
    -    value: originalUserAgent
    -  })
    +  spy.mockRestore()

    The testUserAgent function modifies window.navigator.userAgent but doesn't
    properly restore it after the test. The current restoration only sets the value
    property but doesn't preserve the original property descriptor configuration.
    This could cause side effects between tests.

    apps/platform/tests/hooks/use-is-apple-device.test.ts [13-18]

    -Object.defineProperties(window.navigator, {
    -  userAgent: {
    -    value: userAgent,
    -    writable: true
    -  }
    -})
    +const originalNavigator = Object.getOwnPropertyDescriptor(window, 'navigator');
    +Object.defineProperty(window, 'navigator', {
    +  value: { ...window.navigator, userAgent },
    +  configurable: true
    +});
     
    +// Test code here
    +
    +// Restore the original navigator
    +if (originalNavigator) {
    +  Object.defineProperty(window, 'navigator', originalNavigator);
    +}
    +

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a flaw in the test's mocking logic. The current implementation modifies window.navigator.userAgent but the restoration logic is incomplete, as it doesn't restore the original property descriptor. This could lead to side effects and flaky tests. The suggested approach is more robust for ensuring test isolation.

    Medium
    Remove ESM module type

    The "type": "module" setting may cause compatibility issues with Jest. Jest has
    specific requirements for module resolution, and setting the package type to ESM
    can lead to test failures. Consider removing this setting or configuring Jest
    specifically for ESM support.

    apps/platform/package.json [5]

    -"type": "module",
    +// Remove the "type": "module" line entirely
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a potential compatibility issue between Jest and the newly added "type": "module" setting. While Jest can be configured for ESM, it often requires careful setup, and this change could lead to failing tests. This is a valuable, proactive suggestion to prevent potential configuration problems.

    Medium
    Fix incorrect user agent

    The test for Edge on Mac uses an incorrect approach by replacing 'Safari' with
    'Edg' in the Safari user agent string. This doesn't properly represent an Edge
    browser on Mac and could lead to false test results.

    apps/platform/tests/hooks/use-is-apple-device.test.ts [148-150]

     it('should return true for Edge on Mac', () => {
    -  testUserAgent(osAgentPairs.mac.safari.replace('Safari', 'Edg'), true)
    +  testUserAgent('Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36 Edg/125.0.0.0', true)
     })
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that the user agent string for 'Edge on Mac' is being generated inaccurately. Using a simple string replacement on the Safari user agent does not produce a realistic string for Edge. The proposed user agent string is more accurate, which makes the test case more valid and reliable.

    Low
    • Update

    Copy link

    @Copilot Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull Request Overview

    This PR sets up Jest testing for the @keyshade/platform app and adds comprehensive unit tests for the useIsAppleDevice hook.

    • Configure Jest with ts-jest, a custom jest.config.ts, and module name mappings.
    • Update tsconfig.json and package.json in the platform app to include test files and testing dependencies.
    • Add a new test suite for the useIsAppleDevice hook covering various user-agent strings.

    Reviewed Changes

    Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

    Show a summary per file
    File Description
    package.json (root) Updated test:platform script to run without dependencies.
    apps/platform/tsconfig.json Included __tests__/**/*.ts and jest.config.ts in compilation.
    apps/platform/package.json Added type: "module", testing scripts, and Jest-related deps.
    apps/platform/jest.config.ts New Jest config using ts-jest, jsdom, and custom mappings.
    apps/platform/tests/hooks/use-is-apple-device.test.ts New test file covering useIsAppleDevice against various UAs.
    Comments suppressed due to low confidence (1)

    apps/platform/tests/hooks/use-is-apple-device.test.ts:33

    • [nitpick] The keys in osAgentPairs mix casing (e.g. mac, iPad, androidTablet); consider using consistent lowercase naming to avoid confusion.
    const osAgentPairs = {
    

    },
    moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
    coverageDirectory: '../../coverage/apps/workspace'
    testMatch: ['**/__tests__/**/*.(spec|test).(ts|tsx)'],
    Copy link
    Preview

    Copilot AI Jun 22, 2025

    Choose a reason for hiding this comment

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

    Consider adding a coverageDirectory field (e.g. coverage/apps/platform) so that test coverage reports are consistently generated and stored.

    Copilot uses AI. Check for mistakes.

    @kriptonian1 kriptonian1 changed the title feat: unit test for custom hooks [WIP]: unit test for custom hooks Jun 23, 2025
    @kriptonian1 kriptonian1 marked this pull request as draft June 23, 2025 14:18
    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Jun 23, 2025

    CI Feedback 🧐

    (Feedback updated until commit 276cfae)

    A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

    Action: Validate PR title

    Failed stage: Lint PR [❌]

    Failure summary:

    The action failed because the PR title "[WIP]: unit test for custom hooks" does not follow the
    conventional commits format. The action amannn/action-semantic-pull-request@v5 requires PR titles to
    include a valid release type prefix according to the Conventional Commits specification.

    Relevant error logs:
    1:  ##[group]Runner Image Provisioner
    2:  Hosted Compute Agent
    ...
    
    31:  SecurityEvents: write
    32:  Statuses: write
    33:  ##[endgroup]
    34:  Secret source: Actions
    35:  Prepare workflow directory
    36:  Prepare all required actions
    37:  Getting action download info
    38:  Download action repository 'amannn/action-semantic-pull-request@v5' (SHA:0723387faaf9b38adef4775cd42cfd5155ed6017)
    39:  Complete job name: Validate PR title
    40:  ##[group]Run amannn/action-semantic-pull-request@v5
    41:  with:
    42:  githubBaseUrl: https://api.github.com
    43:  env:
    44:  GITHUB_TOKEN: ***
    45:  ##[endgroup]
    46:  ##[error]No release type found in pull request title "[WIP]: unit test for custom hooks". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/
    47:  
    

    Copy link
    Contributor

    @kriptonian1, please resolve all open reviews!

    Copy link
    Contributor

    @kriptonian1, please resolve all open reviews; otherwise this PR will be closed after Sun Jun 29 2025 19:58:27 GMT+0000 (Coordinated Universal Time)!

    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