-
Notifications
You must be signed in to change notification settings - Fork 12
feat/moreTestingUpdates #663
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
Changes from all commits
b2f1db7
663b23f
9ed9751
3d11c14
3252141
56664e6
542004f
2f207ed
9d89fdb
c35f1f2
9012b81
2ea844a
ab57df9
1bf951e
2524e46
e5b35c8
29346f3
4b4d829
5eee466
810345b
fdef447
3fa5194
fd25aac
5f2894d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,78 @@ | ||
| name: Test | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| push: | ||
| branches: | ||
| - main | ||
|
|
||
| concurrency: ${{ github.workflow }}-${{ github.ref }} | ||
| concurrency: | ||
| group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Test | ||
| timeout-minutes: 20 | ||
| determine-packages: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| packages: ${{ steps.set-packages.outputs.packages }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Get package names | ||
| id: set-packages | ||
| run: | | ||
| SEARCH_DIRS="plugins utils validate convert enrich import export" | ||
|
|
||
| PACKAGES=$( | ||
| for dir in $SEARCH_DIRS; do | ||
| if [ -d "$dir" ]; then | ||
| find "$dir" -maxdepth 2 -name "package.json" -exec sh -c ' | ||
| PACKAGE_NAME=$(jq -r .name {}) | ||
| if [ "$PACKAGE_NAME" != "null" ]; then | ||
| echo "$PACKAGE_NAME" | ||
| fi | ||
| ' \; | ||
| fi | ||
| done | jq -R -s -c 'split("\n")[:-1]' | ||
| ) | ||
|
|
||
| echo "packages=$PACKAGES" >> $GITHUB_OUTPUT | ||
| echo "Found packages: $PACKAGES" | ||
|
|
||
| test-packages: | ||
| needs: determine-packages | ||
| name: ${{ matrix.package }} | ||
| runs-on: ubuntu-latest | ||
| if: ${{ github.head_ref != 'changeset-release/main' }} | ||
| strategy: | ||
| matrix: | ||
| package: ${{fromJson(needs.determine-packages.outputs.packages)}} | ||
| fail-fast: false | ||
| steps: | ||
| - name: Check out code | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| fetch-depth: 2 | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js environment | ||
| uses: actions/setup-node@v3 | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 16 | ||
| node-version: 18 | ||
| cache: 'npm' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Build | ||
| run: npx turbo run build | ||
|
|
||
| - name: Test | ||
| run: npm run test | ||
| run: npx turbo run test --filter=${{ matrix.package }} | ||
|
|
||
| test: | ||
| needs: test-packages | ||
| name: Test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check test results | ||
| run: | | ||
| echo "All package tests completed successfully" | ||
| exit 0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Issue Detected: The
Please ensure that the 🔗 Analysis chainLGTM: Environment setup looks good. The setupFiles configuration correctly includes the dotenv configuration file. This ensures that environment variables are properly loaded before tests run. Please verify that the file path '../../test/dotenv-config.js' is correct and the file exists: [/run_scripts] 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Verify the existence of the dotenv configuration file
if [ -f "test/dotenv-config.js" ]; then
echo "dotenv-config.js file exists"
else
echo "dotenv-config.js file not found"
fi
Length of output: 195 Script: #!/bin/bash
# Verify the existence of the dotenv configuration file with the correct path
if [ -f "../../test/dotenv-config.js" ]; then
echo "dotenv-config.js file exists"
else
echo "dotenv-config.js file not found"
fi
Length of output: 113 |
||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 90_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider alternatives to forceExit. While Consider using |
||
| passWithNoTests: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reconsider passWithNoTests setting. While Consider setting this to |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider localizing the global setup script. The global setup script is currently located outside the package directory. For better maintainability and to ensure package-specific setup, consider creating a local copy of this setup script within the package. This approach would allow for package-specific customizations without affecting other packages. |
||
| forceExit: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reconsider using forceExit. Setting Consider the following alternatives:
This approach will help maintain cleaner and more reliable tests in the long run. |
||
| passWithNoTests: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reconsider using passWithNoTests. Setting Consider the following alternatives:
This approach will help maintain better test coverage and prevent overlooking missing tests. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider removing While Consider removing this option and instead focus on proper cleanup in your tests and setup/teardown scripts. If you're having issues with Jest not exiting cleanly, it's often a sign of unhandled promises or timers that haven't been cleared. If you decide to keep this option, add a comment explaining why it's necessary for this specific plugin. |
||
| passWithNoTests: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reconsider While Consider removing this option to ensure that all modules have at least one test. If you decide to keep it, add a comment explaining why it's necessary for this specific plugin and perhaps set a reminder to remove it once tests are in place. To help ensure comprehensive test coverage, you could run the following script to check for test files corresponding to your source files: #!/bin/bash
# Description: Check for missing test files
echo "Checking for source files without corresponding test files:"
for src_file in $(find src -name '*.ts' -o -name '*.js'); do
test_file="${src_file/src/test}"
test_file="${test_file%.ts}.test.ts"
test_file="${test_file%.js}.test.js"
if [ ! -f "$test_file" ]; then
echo "Missing test file for: $src_file"
fi
done |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Inconsistency Detected in The
Please verify if the extended timeout for these plugins is necessary. If not, consider standardizing the 🔗 Analysis chainApproved: Test timeout set to 60 seconds. The 60-second test timeout is appropriate for database operations, which may take longer than typical unit tests. This aligns with the PR objectives mentioning changes to test timing. To ensure this timeout is consistent across the project, let's check other Jest configurations: Action Required: Verify The previous check did not locate This will help confirm whether the 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for consistent test timeout across Jest configurations
# Test: Search for testTimeout in Jest config files
rg --type js --type ts 'testTimeout' -g '*jest.config.*'
Length of output: 4496 Script: #!/bin/bash
# Description: Verify the presence of 'testTimeout' in all Jest configuration files
rg --type js --type ts 'testTimeout' -g '*jest.config.*'
Length of output: 2170 |
||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
| forceExit: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reconsider using While
If you decide to keep forceExit: true, // Needed due to [specific reason, e.g., "known issue with library X"] |
||
| passWithNoTests: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reconsider using While
If you decide to keep passWithNoTests: true, // Temporary setting while tests are being developed |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| module.exports = { | ||
| testEnvironment: 'node', | ||
|
|
||
| transform: { | ||
| '^.+\\.tsx?$': 'ts-jest', | ||
| }, | ||
| setupFiles: ['../../test/dotenv-config.js'], | ||
| setupFilesAfterEnv: [ | ||
| '../../test/betterConsoleLog.js', | ||
| '../../test/unit.cleanup.js', | ||
| ], | ||
| testTimeout: 60_000, | ||
| globalSetup: '../../test/setup-global.js', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider using path.resolve() for globalSetup The globalSetup configuration is good for setting up the test environment. However, to improve portability, consider using const path = require('path');
module.exports = {
// ... other configs
globalSetup: path.resolve(__dirname, '../../test/setup-global.js'),
// ... remaining configs
}This change will make the configuration more resilient to changes in file structure. |
||
| forceExit: true, | ||
| passWithNoTests: true, | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the implications of forceExit and passWithNoTests.
While these configurations can be useful, they come with potential drawbacks:
forceExit: trueforces Jest to exit after all tests complete. This might hide issues with asynchronous operations or memory leaks.passWithNoTests: trueallows the test suite to pass when no tests are found. This can be useful during development but might hide issues in CI/CD pipelines.Consider the following alternatives:
forceExit, ensure all asynchronous operations are properly closed/resolved in your tests or afterAll blocks.passWithNoTests, consider adding a check in your CI/CD pipeline to ensure that tests exist and are being run.If you decide to keep these settings, please add comments explaining the rationale to prevent future confusion.