test(e2e): modernize plugin e2e harness and dist publish flow#197
test(e2e): modernize plugin e2e harness and dist publish flow#197
Conversation
📝 WalkthroughWalkthroughAdds isolated e2e test workspace utilities and refactors tests to run from dynamically created project directories; updates async command helpers to require explicit cwd; adds environment config and release publish target for the nx-forge package; moves and replaces several e2e tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner (Jest)
participant Workspace as Test Workspace (created by createTestProject)
participant NxCLI as Nx CLI (npx nx)
participant ForgeCLI as Forge CLI (npx forge)
participant GraphQL as Atlassian GraphQL API
participant FS as File System
TestRunner->>Workspace: createTestProject()
Note right of Workspace: workspace created, nx.json analytics=false
TestRunner->>NxCLI: run generate (cwd=Workspace)
NxCLI->>FS: write generated app files (manifest.yml, src, etc.)
TestRunner->>NxCLI: run build/package/register/deploy (cwd=Workspace)
NxCLI->>FS: produce dist/apps/<app>/manifest.yml
TestRunner->>ForgeCLI: install/uninstall (cwd=dist app)
ForgeCLI->>GraphQL: register/delete app
GraphQL-->>FS: return registered app data (update manifest)
TestRunner->>Workspace: cleanupTestProject()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/nx-forge-e2e/src/basic-setup.spec.ts (1)
56-167:⚠️ Potential issue | 🟠 MajorMove the Forge uninstall/delete work into a
finallyblock.Once this test registers the app, every later assertion/command is on the happy path. A failure anywhere after that leaves the app installed/registered in the shared Forge environment because
afterAllonly cleans up the local workspace. Please trackappIdand do best-effort uninstall +deleteApp(appId)in afinallyso cleanup still happens on partial failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` around lines 56 - 167, Wrap the post-registration flow (everything after you obtain appId) in a try/finally: after you parse appId from registeredOutputManifestContent, enter try { /* existing deploy/install/assertions */ } finally { if (appId) { attempt a best-effort uninstall by calling runForgeCommandAsync with the same flags and cwd used for install (the previous uninstallResult call), and then call deleteApp(appId)(apiClient) to remove the registered app; swallow/log (don't throw) any errors from these cleanup steps so failures during test assertions don't leave the app registered. Move the existing uninstall + deleteApp logic into that finally block and ensure appId is defined in the outer scope so the finally can reference it. }
🧹 Nitpick comments (2)
.codex/environments/environment.toml (1)
5-6: Consider usingnpm cifor reproducible installs.For test/CI environments,
npm ciis generally preferred overnpm ibecause it:
- Provides reproducible builds by strictly following
package-lock.json- Installs faster by skipping certain validation steps
- Fails if
package.jsonand lock file are out of sync, catching potential issues early♻️ Proposed change
[setup] -script = "npm i" +script = "npm ci"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/environments/environment.toml around lines 5 - 6, Update the setup script under the [setup] section to use a reproducible CI install command: replace the current script value "npm i" with "npm ci" so the environment uses the lockfile for deterministic, faster installs and fails when package.json and package-lock.json are out of sync; ensure the change is applied to the script key in the [setup] block.e2e/nx-forge-e2e/src/utils/async-commands.ts (1)
29-35: Remove the repo-root fallback ifcwdis meant to be mandatory.The type says
cwdis required, but these default option objects still let future call sites omit it and silently run againstprocess.cwd()again. If workspace isolation is the goal here, makeoptsrequired and force every caller to passcwd.Also applies to: 63-69, 90-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/utils/async-commands.ts` around lines 29 - 35, The function runCommandAsync currently provides a default opts object which falls back to process.cwd(), contradicting the declared requirement that cwd is mandatory; remove the default opts value and make the opts parameter required (so callers must supply opts.cwd), update any similar helper functions in this file (the other functions around lines 63-69 and 90-96) to require opts.cwd as well, and fix all call sites to pass an explicit cwd; ensure type signatures reflect opts as required (no default) and remove any internal fallback to process.cwd().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex/environments/environment.toml:
- Line 1: Remove the misleading autogenerated comment at the top of
.codex/environments/environment.toml (delete the line "# THIS IS AUTOGENERATED.
DO NOT EDIT MANUALLY"), then either add this environment to .codex/config.toml
so it will be activated (reference the environment identifier used inside
environment.toml) or add a short comment in environment.toml explaining why it's
intentionally optional; finally, in the environment's setup script replace any
occurrence of "npm i" with "npm ci" to ensure reproducible installs.
In `@e2e/nx-forge-e2e/src/utils/test-project.ts`:
- Around line 30-33: The runCommand invocation that currently calls
`create-nx-workspace@latest` should be changed to pin the Nx release used for
test workspaces to 22.6.1; locate the `runCommand` call that executes `npx -y
create-nx-workspace@latest ${projectName} ...` in test-project.ts and replace
`@latest` with `@22.6.1` so the created workspace's `create-nx-workspace`/`nx`
versions match the repo's pinned Nx dependencies and avoid version skew in e2e
tests.
---
Outside diff comments:
In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts`:
- Around line 56-167: Wrap the post-registration flow (everything after you
obtain appId) in a try/finally: after you parse appId from
registeredOutputManifestContent, enter try { /* existing
deploy/install/assertions */ } finally { if (appId) { attempt a best-effort
uninstall by calling runForgeCommandAsync with the same flags and cwd used for
install (the previous uninstallResult call), and then call
deleteApp(appId)(apiClient) to remove the registered app; swallow/log (don't
throw) any errors from these cleanup steps so failures during test assertions
don't leave the app registered. Move the existing uninstall + deleteApp logic
into that finally block and ensure appId is defined in the outer scope so the
finally can reference it. }
---
Nitpick comments:
In @.codex/environments/environment.toml:
- Around line 5-6: Update the setup script under the [setup] section to use a
reproducible CI install command: replace the current script value "npm i" with
"npm ci" so the environment uses the lockfile for deterministic, faster installs
and fails when package.json and package-lock.json are out of sync; ensure the
change is applied to the script key in the [setup] block.
In `@e2e/nx-forge-e2e/src/utils/async-commands.ts`:
- Around line 29-35: The function runCommandAsync currently provides a default
opts object which falls back to process.cwd(), contradicting the declared
requirement that cwd is mandatory; remove the default opts value and make the
opts parameter required (so callers must supply opts.cwd), update any similar
helper functions in this file (the other functions around lines 63-69 and 90-96)
to require opts.cwd as well, and fix all call sites to pass an explicit cwd;
ensure type signatures reflect opts as required (no default) and remove any
internal fallback to process.cwd().
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d124c9e5-3abb-4260-804e-69e239ec8c8f
📒 Files selected for processing (12)
.codex/environments/environment.tomle2e/nx-forge-e2e/project.jsone2e/nx-forge-e2e/src/application.generator.spec.tse2e/nx-forge-e2e/src/basic-setup.spec.tse2e/nx-forge-e2e/src/utils/async-commands.tse2e/nx-forge-e2e/src/utils/atlassian-graphql-client.tse2e/nx-forge-e2e/src/utils/config.tse2e/nx-forge-e2e/src/utils/generate-forge-app.tse2e/nx-forge-e2e/src/utils/test-project.tse2e/nx-forge-e2e/tests/application.generator.spec.tse2e/nx-forge-e2e/tests/utils/e2e-workspace.tspackages/nx-forge/project.json
💤 Files with no reviewable changes (2)
- e2e/nx-forge-e2e/tests/utils/e2e-workspace.ts
- e2e/nx-forge-e2e/tests/application.generator.spec.ts
| @@ -0,0 +1,6 @@ | |||
| # THIS IS AUTOGENERATED. DO NOT EDIT MANUALLY | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for scripts or documentation that generate environment.toml
# Search for references to generating or creating this environment file
rg -i "environment\.toml|generate.*environment" -g '!.codex/environments/*' -C3
# Look for any .codex documentation
fd -e md -e txt . .codex/Repository: toolsplus/nx-forge
Length of output: 44
🏁 Script executed:
# Explore .codex directory structure
find .codex -type f -o -type d | head -20
# Search for any setup, build, or generation scripts
fd -e sh -e js -e py -e ts -e bash | xargs rg -l "environment|generate" | head -20
# Check .codex/config.toml if it exists
if [ -f ".codex/config.toml" ]; then
echo "=== .codex/config.toml ===";
cat .codex/config.toml;
fi
# Look for any scripts in the repo root or .codex
ls -la .codex/*.* 2>/dev/null || true
ls -la *.js *.json *.sh 2>/dev/null || trueRepository: toolsplus/nx-forge
Length of output: 1837
🏁 Script executed:
# View the environment.toml file content
echo "=== .codex/environments/environment.toml ==="
cat .codex/environments/environment.toml
# Search for any references to this environment or "nx-forge"
echo -e "\n=== References to nx-forge environment ==="
rg -i "nx-forge|environments" --type toml
# Search for environment setup/configuration in docs or comments
echo -e "\n=== Environment-related documentation ==="
fd -e md -e txt | xargs rg -l -i "environment|codex" 2>/dev/null | head -10Repository: toolsplus/nx-forge
Length of output: 533
🏁 Script executed:
# Check the documentation files mentioned
echo "=== docs/guides/getting-started.md (first 100 lines) ==="
head -100 docs/guides/getting-started.md
echo -e "\n=== docs/guides/transforming-the-manifest.md (first 100 lines) ==="
head -100 docs/guides/transforming-the-manifest.md
# Search for any Codex environment patterns or examples
echo -e "\n=== References to environment setup ==="
rg -i "environment|setup.*script" docs/ | head -20Repository: toolsplus/nx-forge
Length of output: 10690
Remove the autogenerated comment—this file is manually maintained.
The comment on line 1 claims the file is autogenerated, but no generation script or tool was found in the codebase. Since this environment is being manually created and maintained in the PR, the misleading comment should be removed to avoid confusing future maintainers.
Additionally, this environment is defined but not referenced in .codex/config.toml, so it won't be activated. Either activate it in the main config or document why this environment definition is needed if it's meant to be optional.
Consider using npm ci instead of npm i in the setup script for reproducible, deterministic builds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codex/environments/environment.toml at line 1, Remove the misleading
autogenerated comment at the top of .codex/environments/environment.toml (delete
the line "# THIS IS AUTOGENERATED. DO NOT EDIT MANUALLY"), then either add this
environment to .codex/config.toml so it will be activated (reference the
environment identifier used inside environment.toml) or add a short comment in
environment.toml explaining why it's intentionally optional; finally, in the
environment's setup script replace any occurrence of "npm i" with "npm ci" to
ensure reproducible installs.
| runCommand( | ||
| `npx -y create-nx-workspace@latest ${projectName} --preset=apps --nxCloud=skip --packageManager=npm --no-interactive`, | ||
| TEST_PROJECTS_ROOT | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(package\.json|nx\.json)" | head -20Repository: toolsplus/nx-forge
Length of output: 155
🏁 Script executed:
cat e2e/nx-forge-e2e/src/utils/test-project.ts | head -50Repository: toolsplus/nx-forge
Length of output: 1470
🏁 Script executed:
find . -name "package.json" -type f | head -5 | xargs grep -l "nx" 2>/dev/nullRepository: toolsplus/nx-forge
Length of output: 109
🏁 Script executed:
cat package.json | grep -A 2 -B 2 '"nx"'Repository: toolsplus/nx-forge
Length of output: 311
🏁 Script executed:
cat packages/nx-forge/package.json | grep -A 2 -B 2 '"nx"'Repository: toolsplus/nx-forge
Length of output: 44
🏁 Script executed:
rg "create-nx-workspace" e2e/ --type ts --type jsRepository: toolsplus/nx-forge
Length of output: 227
🏁 Script executed:
rg "create-nx-workspace" --type ts --type js -A 2 -B 2Repository: toolsplus/nx-forge
Length of output: 442
🏁 Script executed:
cat packages/nx-forge/package.json | head -30Repository: toolsplus/nx-forge
Length of output: 813
🏁 Script executed:
find e2e -name "*.ts" -o -name "*.js" | xargs grep -l "workspace\|version" | head -10Repository: toolsplus/nx-forge
Length of output: 104
🏁 Script executed:
rg "nx\s+version\|create-nx-workspace" --type md --type ts --type js -i | head -20Repository: toolsplus/nx-forge
Length of output: 44
Pin create-nx-workspace to match the repo's Nx version (22.6.1) instead of using @latest.
Line 31 creates the test workspace with an unpinned create-nx-workspace@latest, while the plugin is built against the repo's fixed Nx dependencies (all pinned to 22.6.1). This version skew causes e2e test failures from upstream Nx releases rather than actual regressions. Nx's create-nx-workspace, nx, and @nx/* packages are released in lockstep and must use matching versions. Change to create-nx-workspace@22.6.1 to keep the test harness aligned with the plugin's build environment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nx-forge-e2e/src/utils/test-project.ts` around lines 30 - 33, The
runCommand invocation that currently calls `create-nx-workspace@latest` should
be changed to pin the Nx release used for test workspaces to 22.6.1; locate the
`runCommand` call that executes `npx -y create-nx-workspace@latest
${projectName} ...` in test-project.ts and replace `@latest` with `@22.6.1` so
the created workspace's `create-nx-workspace`/`nx` versions match the repo's
pinned Nx dependencies and avoid version skew in e2e tests.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
tbinna
force-pushed
the
modernize-e2e-test-setup
branch
from
1cf72a0 to
d4f9022
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
e2e/nx-forge-e2e/src/basic-setup.spec.ts (3)
32-54: Consider adding an explicit Jest timeout for this long-running e2e test.This test performs generate, build, package, register, deploy, and install operations sequentially, which could exceed Jest's default 5-second timeout. E2e tests typically need longer timeouts.
💡 Suggested timeout configuration
describe('Forge lifecycle', () => { + jest.setTimeout(300_000); // 5 minutes for full lifecycle + // initialize before all tests let projectDirectory: string;Alternatively, set it in the Jest config for the e2e project if not already configured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` around lines 32 - 54, This e2e spec can exceed Jest's default timeout; add an explicit longer Jest timeout (e.g., jest.setTimeout(300000)) at the top of this test file (or at the start of beforeAll) so long-running steps (createTestProject, runCommandAsync, runNxCommandAsync, cleanupTestProject) won't be aborted; ensure the timeout call is placed before beforeAll runs so afterAll cleanup still executes as expected.
64-81: Strict stderr assertions may cause flaky tests.Assertions like
expect(nxBuildResult.stderr).toEqual('')could fail intermittently if Nx or underlying tools emit warnings or deprecation notices to stderr (which many CLI tools do even on successful runs). Consider whether relaxing these checks is appropriate.💡 Alternative: assert no error keywords instead of empty stderr
- expect(nxBuildResult.stderr).toEqual(''); + // Allow non-critical stderr output (warnings, deprecations) + expect(nxBuildResult.stderr).not.toMatch(/error|failed/i);Or simply remove the stderr assertions if the stdout success message is sufficient validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` around lines 64 - 81, The tests currently assert exact empty stderr for runNxCommandAsync results (nxBuildResult.stderr and nxPackageResult.stderr), which can be flaky; update the assertions in basic-setup.spec.ts to relax these checks by either removing the strict expect(...).toEqual('') assertions or replacing them with non-failing checks such as asserting stderr does not contain error keywords (e.g., expect(stripAnsi(nxBuildResult.stderr).toLowerCase()).not.toContain('error')) and do the same for nxPackageResult, keeping the existing stdout success assertions (stripAnsi(...).toContain / expect.stringContaining) and using runNxCommandAsync and stripAnsi to locate the failing assertions to modify.
113-117: Redundant assertions can be consolidated.If the regex matches,
registeredAppIdwill be a non-empty string. If it doesn't match,registeredAppIdwill beundefined. The three assertions can be simplified.💡 Simplified assertion
const [registeredAppId] = registeredOutputManifestContent.match(registeredAppIdRegex) ?? []; - expect(registeredAppId).not.toBeNull(); - expect(registeredAppId).toBeDefined(); - expect(registeredAppId).not.toEqual(''); + expect(registeredAppId).toMatch(registeredAppIdRegex);This single assertion validates that
registeredAppIdexists and matches the expected format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` around lines 113 - 117, The three redundant assertions around registeredAppId should be consolidated: locate where registeredAppId is derived from registeredOutputManifestContent.match(registeredAppIdRegex) and replace the multiple expect calls with a single assertion that both verifies presence and format, e.g., assert registeredAppId against registeredAppIdRegex (use expect(registeredAppId).toMatch(registeredAppIdRegex) or equivalent). Ensure you keep the same variables (registeredAppId, registeredOutputManifestContent, registeredAppIdRegex) so the test still fails when there's no match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts`:
- Line 128: Fix the typo in the inline comment that reads "// Run with
`--no-verfiy` because the generated blank app template causes linting errors" by
changing "verfiy" to "verify" so the comment reads "// Run with `--no-verify`
because the generated blank app template causes linting errors"; update the
comment where it appears in basic-setup.spec.ts (the test comment near the e2e
setup block) to ensure consistency.
- Around line 169-173: The warning uses result.errors (an array of objects)
directly which prints as [object Object]; update the log to format the errors,
e.g. map the array to messages or stringify it before interpolating so the
warning shows useful details. Locate the block referencing result and
registeredAppId in basic-setup.spec.ts (the if (!result.success) branch) and
replace the template interpolation with a formatted string such as
result.errors.map(e => e.message).join(', ') or JSON.stringify(result.errors)
when constructing the console.warn message.
---
Nitpick comments:
In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts`:
- Around line 32-54: This e2e spec can exceed Jest's default timeout; add an
explicit longer Jest timeout (e.g., jest.setTimeout(300000)) at the top of this
test file (or at the start of beforeAll) so long-running steps
(createTestProject, runCommandAsync, runNxCommandAsync, cleanupTestProject)
won't be aborted; ensure the timeout call is placed before beforeAll runs so
afterAll cleanup still executes as expected.
- Around line 64-81: The tests currently assert exact empty stderr for
runNxCommandAsync results (nxBuildResult.stderr and nxPackageResult.stderr),
which can be flaky; update the assertions in basic-setup.spec.ts to relax these
checks by either removing the strict expect(...).toEqual('') assertions or
replacing them with non-failing checks such as asserting stderr does not contain
error keywords (e.g.,
expect(stripAnsi(nxBuildResult.stderr).toLowerCase()).not.toContain('error'))
and do the same for nxPackageResult, keeping the existing stdout success
assertions (stripAnsi(...).toContain / expect.stringContaining) and using
runNxCommandAsync and stripAnsi to locate the failing assertions to modify.
- Around line 113-117: The three redundant assertions around registeredAppId
should be consolidated: locate where registeredAppId is derived from
registeredOutputManifestContent.match(registeredAppIdRegex) and replace the
multiple expect calls with a single assertion that both verifies presence and
format, e.g., assert registeredAppId against registeredAppIdRegex (use
expect(registeredAppId).toMatch(registeredAppIdRegex) or equivalent). Ensure you
keep the same variables (registeredAppId, registeredOutputManifestContent,
registeredAppIdRegex) so the test still fails when there's no match.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f15ebec7-856d-4637-8dac-5d7948a24259
📒 Files selected for processing (13)
.codex/environments/environment.tomle2e/nx-forge-e2e/project.jsone2e/nx-forge-e2e/src/application.generator.spec.tse2e/nx-forge-e2e/src/basic-setup.spec.tse2e/nx-forge-e2e/src/utils/async-commands.tse2e/nx-forge-e2e/src/utils/atlassian-graphql-client.tse2e/nx-forge-e2e/src/utils/config.tse2e/nx-forge-e2e/src/utils/generate-forge-app.tse2e/nx-forge-e2e/src/utils/test-project.tse2e/nx-forge-e2e/tests/application.generator.spec.tse2e/nx-forge-e2e/tests/basic-setup.spec.tse2e/nx-forge-e2e/tests/utils/e2e-workspace.tspackages/nx-forge/project.json
💤 Files with no reviewable changes (3)
- e2e/nx-forge-e2e/tests/utils/e2e-workspace.ts
- e2e/nx-forge-e2e/tests/application.generator.spec.ts
- e2e/nx-forge-e2e/tests/basic-setup.spec.ts
✅ Files skipped from review due to trivial changes (3)
- .codex/environments/environment.toml
- e2e/nx-forge-e2e/src/application.generator.spec.ts
- packages/nx-forge/project.json
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/nx-forge-e2e/project.json
- e2e/nx-forge-e2e/src/utils/generate-forge-app.ts
- e2e/nx-forge-e2e/src/utils/test-project.ts
- e2e/nx-forge-e2e/src/utils/async-commands.ts
| try { | ||
| // Deploy | ||
|
|
||
| // Run with `--no-verfiy` because the generated blank app template causes linting errors |
There was a problem hiding this comment.
Typo in comment: "verfiy" should be "verify".
✏️ Fix typo
- // Run with `--no-verfiy` because the generated blank app template causes linting errors
+ // Run with `--no-verify` because the generated blank app template causes linting errors📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Run with `--no-verfiy` because the generated blank app template causes linting errors | |
| // Run with `--no-verify` because the generated blank app template causes linting errors |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` at line 128, Fix the typo in the
inline comment that reads "// Run with `--no-verfiy` because the generated blank
app template causes linting errors" by changing "verfiy" to "verify" so the
comment reads "// Run with `--no-verify` because the generated blank app
template causes linting errors"; update the comment where it appears in
basic-setup.spec.ts (the test comment near the e2e setup block) to ensure
consistency.
| if (!result.success) { | ||
| console.warn( | ||
| `Failed to delete registered app ${registeredAppId}: ${result.errors}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Error array won't format properly in the warning message.
result.errors is an array of objects [{ message: string }]. Using it directly in a template literal will output [object Object] instead of useful information.
✏️ Fix error formatting
if (!result.success) {
console.warn(
- `Failed to delete registered app ${registeredAppId}: ${result.errors}`
+ `Failed to delete registered app ${registeredAppId}: ${result.errors?.map(e => e.message).join(', ')}`
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!result.success) { | |
| console.warn( | |
| `Failed to delete registered app ${registeredAppId}: ${result.errors}` | |
| ); | |
| } | |
| if (!result.success) { | |
| console.warn( | |
| `Failed to delete registered app ${registeredAppId}: ${result.errors?.map(e => e.message).join(', ')}` | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nx-forge-e2e/src/basic-setup.spec.ts` around lines 169 - 173, The warning
uses result.errors (an array of objects) directly which prints as [object
Object]; update the log to format the errors, e.g. map the array to messages or
stringify it before interpolating so the warning shows useful details. Locate
the block referencing result and registeredAppId in basic-setup.spec.ts (the if
(!result.success) branch) and replace the template interpolation with a
formatted string such as result.errors.map(e => e.message).join(', ') or
JSON.stringify(result.errors) when constructing the console.warn message.
1 participant
Summary by CodeRabbit
Tests
Chores