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

test(secret-scan): Add test coverage #534

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

Souvik9205
Copy link
Contributor

@Souvik9205 Souvik9205 commented Nov 13, 2024

User description

Description

This PR fixes a test coverage-related issue in the secret-scan package by migrating from Mocha to Jest for improved test execution and flexibility. The changes include:

  • Replacing Mocha configuration with Jest.
  • Updating the workflow configuration(validate-secret-scan.yaml).
  • Adding secret-scan flag on codecov.yml.

Fixes #521

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

PR Type

Tests, Enhancement


Description

  • Migrated the secret-scan package tests from Mocha to Jest for improved test execution and flexibility.
  • Added Jest configuration and updated TypeScript settings for Jest compatibility.
  • Updated dependencies to include Jest and remove Mocha.
  • Modified GitHub Actions workflow to include Jest coverage.
  • Added secret-scan flag to the Codecov configuration for better coverage tracking.

Changes walkthrough 📝

Relevant files
Tests
secret.test.ts
Migrate tests from Mocha to Jest framework                             

packages/secret-scan/src/test/secret.test.ts

  • Migrated from Mocha to Jest for testing.
  • Replaced assert.equal with expect for assertions.
  • Removed Mocha-specific imports.
  • Added missing semicolons for consistency.
  • +59/-12 
    Configuration changes
    jest.config.ts
    Add Jest configuration for secret-scan package                     

    packages/secret-scan/jest.config.ts

  • Added Jest configuration file.
  • Configured TypeScript transformation with ts-jest.
  • Set up coverage collection and reporting.
  • +16/-0   
    package.json
    Update package scripts for Jest and formatting                     

    packages/secret-scan/package.json

  • Changed test script from Mocha to Jest.
  • Updated format script to exclude test directory.
  • +2/-4     
    tsconfig.json
    Update TypeScript configuration for Jest compatibility     

    packages/secret-scan/tsconfig.json

  • Added module resolution and isolated modules options.
  • Included Jest configuration file in TypeScript project.
  • +11/-2   
    tsconfig.spec.json
    Add TypeScript configuration for Jest tests                           

    packages/secret-scan/tsconfig.spec.json

  • Created a new TypeScript configuration for Jest tests.
  • Set module to ES2022 and included Jest types.
  • +14/-0   
    validate-secret-scan.yaml
    Update GitHub Actions workflow for Jest coverage                 

    .github/workflows/validate-secret-scan.yaml

    • Updated workflow to include Jest coverage file.
    +1/-0     
    codecov.yml
    Add secret-scan flag to codecov configuration                       

    codecov.yml

  • Added a new flag secret-scan to the code coverage configuration.
  • Specified paths for the secret-scan package.
  • Set coverage targets for project and patch types under secret-scan.
  • +9/-0     
    Dependencies
    pnpm-lock.yaml
    Update dependencies for Jest migration                                     

    pnpm-lock.yaml

  • Updated dependencies to include Jest and remove Mocha.
  • Adjusted dependency versions and added @swc/helpers.
  • Removed unnecessary packages related to Mocha.
  • +264/-235

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    521 - Fully compliant

    Compliant requirements:

    • Convert test framework from Mocha to Jest
    • Update validate-secret-scan.yaml to upload coverage data
    • Add secret-scan flag in codecov.yml
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Coverage
    Verify that test assertions are properly migrated from assert.equal to Jest's expect().toBe() and that error messages are preserved

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Enable seamless interoperability between ES modules and CommonJS modules in test files

    Add "esModuleInterop" compiler option to ensure compatibility with CommonJS modules
    in Jest tests, as Jest uses CommonJS by default.

    packages/secret-scan/tsconfig.spec.json [1-7]

     {
    -  "extends": "./tsconfig.json",
    +  "extends": "./tsconfig.json", 
       "compilerOptions": {
         "outDir": "dist/out-tsc",
         "module": "ES2022",
    -    "types": ["jest", "node"]
    +    "types": ["jest", "node"],
    +    "esModuleInterop": true
       }
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding "esModuleInterop" is crucial for Jest testing with ES modules, as it prevents potential import/export compatibility issues between ES modules and CommonJS modules, which could cause test failures.

    8
    Add multiple coverage report formats for improved visibility and accessibility

    Add HTML and text coverage reporters alongside JSON for better visibility of test
    coverage in CI/CD pipelines and local development.

    packages/secret-scan/jest.config.ts [14]

    -coverageReporters: ['json'],
    +coverageReporters: ['json', 'html', 'text'],
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Adding HTML and text coverage reporters alongside JSON provides better visibility of test coverage results in different contexts, making it easier to review coverage both locally and in CI/CD pipelines.

    5
    Maintainability
    Include more context in test failure messages to aid debugging

    Add test case index to error message for easier debugging when tests fail. The
    current implementation drops this useful information that was present in the
    original code.

    packages/secret-scan/src/test/secret.test.ts [76-79]

     testcases.forEach(({ input, expected }, index) => {
       const result = secretDetector.detect(input)
    -  expect(result.found).toBe(expected)
    +  expect(result.found).toBe(expected, `(i=${index}) For Input: ${input}, expected ${expected} but got ${result.found}`)
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion restores valuable debugging information that was lost during the migration to Jest, making test failures easier to diagnose by including the test case index and input values in the error message.

    7
    Best practice
    Enable TypeScript project references for better build performance in monorepos

    The composite flag should be set to true since this is a TypeScript project within a
    monorepo. This enables project references and incremental builds.

    packages/secret-scan/tsconfig.json [28]

    -"composite": false
    +"composite": true
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Setting composite to true enables TypeScript project references which is important for build performance and dependency management in monorepos, allowing for incremental compilation and better project organization.

    7

    💡 Need additional feedback ? start a PR chat

    @rajdip-b rajdip-b merged commit 8e618ca into keyshade-xyz:develop Nov 13, 2024
    6 checks passed
    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.

    SECRET-SCAN: Add test coverage
    2 participants