Skip to content

test: improve E2E test cleanup and assertion reliability - #34601

Merged
alan-agius4 merged 2 commits into
angular:mainfrom
alan-agius4:test/e2e-fixes
Oct 30, 2026
Merged

alan-agius4 merged 2 commits into
angular:mainfrom
alan-agius4:test/e2e-fixes

Conversation

@alan-agius4

Copy link
Copy Markdown
Collaborator

This PR addresses several reliability and cleanup issues identified across E2E test suites:

  1. Fix assertion in registers-tools.ts to check stdoutOutsideWorkspace instead of reusing stdoutInsideWorkspace, and restore the working directory on exit.
  2. Add missing await to writeMultipleFiles in ssr-http-requests-assets.ts to avoid a startup race condition.
  3. Ensure child directory removal in component-child-dir.ts restores working directory before rimraf to avoid cleanup failures.
  4. Wrap global configuration modifications in try-finally in config-global.ts and config-global-validation.ts to prevent leaking test configuration files in user homedir.
  5. Add try-finally uninstallation of MCP inspector in ai-tutor.ts and best-practices.ts.
  6. Add error-component-render.js and unix-esbuild-unsupported.js to WEBPACK_IGNORE_TESTS in tests/e2e.bzl.

@alan-agius4
alan-agius4 requested a review from clydin October 30, 2026 08:32
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Oct 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request improves E2E test robustness and cleanup by wrapping configuration changes and tool executions in try-finally blocks, ensuring that temporary files are deleted and global packages are uninstalled. The review feedback points out a critical issue where local developer configurations (~/.angular-config.json) could be permanently deleted during local test runs, recommending a backup and restore mechanism instead. Additionally, it suggests wrapping directory restoration in a try-catch block to guarantee that global package uninstallation is never bypassed.

Comment thread tests/e2e/tests/mcp/registers-tools.ts Resolved
Comment thread tests/e2e.bzl Resolved
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 30, 2026
@alan-agius4
alan-agius4 merged commit ef600f2 into angular:main Oct 30, 2026
39 checks passed
@alan-agius4
alan-agius4 deleted the test/e2e-fixes branch October 30, 2026 16:54
@alan-agius4

Copy link
Copy Markdown
Collaborator Author

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants