Add test coverage for import-export feature UIs#2514
Add test coverage for import-export feature UIs#2514jeradrutnam merged 1 commit intoasgardeo:mainfrom
Conversation
Co-authored-by: Copilot <copilot@github.com>
📝 WalkthroughWalkthroughThis PR adds comprehensive test suites for import-export API hooks ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationValidatePage.test.tsx (1)
97-104: ⚡ Quick winTest name does not match what is asserted.
The case says it “navigates to /welcome on cancel,” but it actually verifies cancel is not rendered when there are no parse errors. Please rename it to reflect the real behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationValidatePage.test.tsx` around lines 97 - 104, Rename the test's description string in the ImportConfigurationValidatePage.test.tsx `it` block so it matches the assertion: change the current "navigates to /welcome on cancel (no errors)" to something like "does not render cancel when there are no parse errors" (the `it` that renders <ImportConfigurationValidatePage /> and asserts the cancel button is not in the document).frontend/apps/thunder-console/src/features/welcome/pages/__tests__/WelcomePage.test.tsx (1)
29-31: ⚡ Quick winHardcoded brand name in test setup/assertion.
At Line 30 (
'Thunder') and the brand-coupled assertion at Line 155, is this hardcoded brand name intentional?
If not, prefer deriving the value from config-driven test data (e.g., sharedproductNamefixture) and asserting against that variable rather than a raw brand token.As per coding guidelines:
**/*.tsx: “Scan for hardcoded occurrences of the string literals `Thunder` or `ThunderID`... flag it and ask: Is this hardcoded brand name intentional?”.Also applies to: 152-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/thunder-console/src/features/welcome/pages/__tests__/WelcomePage.test.tsx` around lines 29 - 31, The test hardcodes the brand name 'Thunder' in the useConfig mock and corresponding assertion in WelcomePage.test.tsx; replace the literal with a shared test variable (e.g., productName) used both when mocking useConfig and when asserting the rendered output so the test derives the brand from config-driven test data; update the useConfig mock call and the assertion(s) that reference 'Thunder' (search for useConfig: () => ({ config: {brand: {product_name: 'Thunder'}}}) and the expectation in the test) to reference the shared productName fixture or local const instead of the raw string.frontend/packages/thunder-contexts/tsconfig.lib.json (1)
2-21: ⚡ Quick winVerify emitted
dist/layout still matches the packageexportspaths.This config affects how TS computes the relative paths for emitted declarations (via
rootDir), and the build script emits declarations todist/. Since dependent packages likely import via specificexportsentries (e.g.,./dist/index.d.ts), it’s worth verifying that the newrootDir: "src"doesn’t change whereindex.d.tsand other declarations land underdist/.After applying the change, run the package build for
thunder-contextsand inspectdist/to confirmindex.d.tsends up at the expected path(s) used by this package’sexports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/thunder-contexts/tsconfig.lib.json` around lines 2 - 21, The change set rootDir to "src" in tsconfig.lib.json may shift emitted .d.ts locations and break package exports; rebuild the thunder-contexts package, inspect dist/ to ensure index.d.ts and other declarations match the package.json "exports" entries, and if they don't, either adjust tsconfig.lib.json (rootDir, outDir, declarationDir, or include) so declarations land at the expected paths or update package.json exports to the new paths; check the compilerOptions.rootDir and compilerOptions.declarationDir specifically and re-run the build until dist/ layout matches the exports consumers rely on.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ExportPage.test.tsx`:
- Around line 28-35: The shared mutable test fixture mockMutationState (a
Partial<UseMutationResult<JSONExportResponse, Error, ExportRequest>> with
mutate: mockMutate) is defined once and mutated across tests causing
order-dependent behavior; refactor by creating a factory or resetting it in a
beforeEach: introduce a createMutationState() helper (or move the
mockMutationState initialization into a beforeEach) that returns a fresh
Partial<UseMutationResult<...>> object with mutate: mockMutate, data: undefined,
isPending: false, isError: false, error: null, and update all tests to call that
factory (or read from the fresh variable) so each test gets an isolated mutation
state; apply the same change for other shared mutable fixtures referenced in the
file (lines ~90-173) to ensure test isolation.
In
`@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationUploadPage.test.tsx`:
- Around line 160-171: Test "shows error when continue is clicked without env
file" in ImportConfigurationUploadPage.test.tsx currently only asserts the
Continue button is disabled after uploading a YAML file and does not click or
assert any error; either rename the test to reflect that it checks the
disabled-state (e.g., "disables continue when only yaml is uploaded") or
implement the intended error-flow by enabling the Continue button
programmatically (or simulating user actions that would enable it), then fire a
click on the button (query by role/name 'common:actions.continue') and assert
the expected validation error message appears (use screen.getByText or similar)
to verify the error path when no .env file is provided.
---
Nitpick comments:
In
`@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationValidatePage.test.tsx`:
- Around line 97-104: Rename the test's description string in the
ImportConfigurationValidatePage.test.tsx `it` block so it matches the assertion:
change the current "navigates to /welcome on cancel (no errors)" to something
like "does not render cancel when there are no parse errors" (the `it` that
renders <ImportConfigurationValidatePage /> and asserts the cancel button is not
in the document).
In
`@frontend/apps/thunder-console/src/features/welcome/pages/__tests__/WelcomePage.test.tsx`:
- Around line 29-31: The test hardcodes the brand name 'Thunder' in the
useConfig mock and corresponding assertion in WelcomePage.test.tsx; replace the
literal with a shared test variable (e.g., productName) used both when mocking
useConfig and when asserting the rendered output so the test derives the brand
from config-driven test data; update the useConfig mock call and the
assertion(s) that reference 'Thunder' (search for useConfig: () => ({ config:
{brand: {product_name: 'Thunder'}}}) and the expectation in the test) to
reference the shared productName fixture or local const instead of the raw
string.
In `@frontend/packages/thunder-contexts/tsconfig.lib.json`:
- Around line 2-21: The change set rootDir to "src" in tsconfig.lib.json may
shift emitted .d.ts locations and break package exports; rebuild the
thunder-contexts package, inspect dist/ to ensure index.d.ts and other
declarations match the package.json "exports" entries, and if they don't, either
adjust tsconfig.lib.json (rootDir, outDir, declarationDir, or include) so
declarations land at the expected paths or update package.json exports to the
new paths; check the compilerOptions.rootDir and compilerOptions.declarationDir
specifically and re-run the build until dist/ layout matches the exports
consumers rely on.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00b57b2b-3d67-45c8-a8f4-0ce0d02c2997
📒 Files selected for processing (8)
frontend/apps/thunder-console/src/features/import-export/api/__tests__/useExportConfiguration.test.tsfrontend/apps/thunder-console/src/features/import-export/api/__tests__/useImportConfiguration.test.tsfrontend/apps/thunder-console/src/features/import-export/pages/__tests__/ExportPage.test.tsxfrontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationUploadPage.test.tsxfrontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationValidatePage.test.tsxfrontend/apps/thunder-console/src/features/welcome/pages/__tests__/CreateProjectPage.test.tsxfrontend/apps/thunder-console/src/features/welcome/pages/__tests__/WelcomePage.test.tsxfrontend/packages/thunder-contexts/tsconfig.lib.json
| let mockMutationState: Partial<UseMutationResult<JSONExportResponse, Error, ExportRequest>> = { | ||
| mutate: mockMutate, | ||
| data: undefined, | ||
| isPending: false, | ||
| isError: false, | ||
| error: null, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Reset mockMutationState per test to avoid order-dependent behavior.
Line 28 defines shared mutable state, and later blocks mutate it incrementally. This can make tests depend on execution order. Initialize a fresh base state in a beforeEach for the suite (or create a createMutationState() helper) so each test is isolated.
Also applies to: 90-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ExportPage.test.tsx`
around lines 28 - 35, The shared mutable test fixture mockMutationState (a
Partial<UseMutationResult<JSONExportResponse, Error, ExportRequest>> with
mutate: mockMutate) is defined once and mutated across tests causing
order-dependent behavior; refactor by creating a factory or resetting it in a
beforeEach: introduce a createMutationState() helper (or move the
mockMutationState initialization into a beforeEach) that returns a fresh
Partial<UseMutationResult<...>> object with mutate: mockMutate, data: undefined,
isPending: false, isError: false, error: null, and update all tests to call that
factory (or read from the fresh variable) so each test gets an isolated mutation
state; apply the same change for other shared mutable fixtures referenced in the
file (lines ~90-173) to ensure test isolation.
| it('shows error when continue is clicked without env file', async () => { | ||
| const user = userEvent.setup(); | ||
| render(<ImportConfigurationUploadPage />); | ||
|
|
||
| // Only select yaml file | ||
| const yamlFile = new File(['key: value'], 'config.yaml', {type: 'text/yaml'}); | ||
| await user.upload(document.getElementById('file-upload') as HTMLInputElement, yamlFile); | ||
|
|
||
| // Manually enable the button by patching disabled - instead verify error shows when submitting | ||
| // The button is disabled without envFile so this tests the validation guard | ||
| expect(screen.getByRole('button', {name: 'common:actions.continue'})).toBeDisabled(); | ||
| }); |
There was a problem hiding this comment.
This test doesn’t validate the behavior described in its title.
Line 160 says it should verify an error when continue is clicked without .env, but the test never clicks continue and only asserts disabled state. Please either rename it to match current behavior or implement the intended error-path assertion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/apps/thunder-console/src/features/import-export/pages/__tests__/ImportConfigurationUploadPage.test.tsx`
around lines 160 - 171, Test "shows error when continue is clicked without env
file" in ImportConfigurationUploadPage.test.tsx currently only asserts the
Continue button is disabled after uploading a YAML file and does not click or
assert any error; either rename the test to reflect that it checks the
disabled-state (e.g., "disables continue when only yaml is uploaded") or
implement the intended error-flow by enabling the Continue button
programmatically (or simulating user actions that would enable it), then fire a
click on the button (query by role/name 'common:actions.continue') and assert
the expected validation error message appears (use screen.getByText or similar)
to verify the error path when no .env file is provided.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2514 +/- ##
==========================================
+ Coverage 85.97% 86.59% +0.61%
==========================================
Files 913 913
Lines 64543 64543
==========================================
+ Hits 55492 55888 +396
+ Misses 7046 6650 -396
Partials 2005 2005
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces comprehensive unit and integration tests for the import/export configuration features in the Thunder Console frontend. The new tests cover both API hooks and UI pages, ensuring robust validation of user interactions, error handling, and integration with navigation and logger utilities.
New test coverage for import/export configuration:
API Hook Tests:
useExportConfigurationhook, covering states such as idle, pending, success, error, and correct HTTP request/headers handling.useImportConfigurationhook, validating idle, pending, success, error states, and ensuring correct request structure and headers.Page Component Tests:
ExportPage, verifying initial export trigger, loading, error, and success states, navigation actions, and correct rendering of child components.ImportConfigurationUploadPage, covering file selection, validation, error display, button enablement, and navigation after successful file upload.Summary by CodeRabbit