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

refactor: remove setup-jest files #2995

Merged
merged 1 commit into from
Feb 28, 2025
Merged

refactor: remove setup-jest files #2995

merged 1 commit into from
Feb 28, 2025

Conversation

ahnpnl
Copy link
Collaborator

@ahnpnl ahnpnl commented Feb 28, 2025

Test plan

Green CI

Does this PR introduce a breaking change?

  • Yes
  • No
  • Use setupZoneTestEnv as a replacement for zone test environment setup
  • Use setupZonelessTestEnv as a replacement for zoneless test environment setup

Other information

N.A.

@ahnpnl ahnpnl requested a review from Copilot February 28, 2025 10:11

Choose a reason for hiding this comment

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

PR Overview

This PR refactors the test environment setup by removing the deprecated setup-jest files and updating tests to use the new setupZoneTestEnv functions. Key changes include:

  • Removal of references to setup-jest.js/mjs from asset lists and test imports.
  • Updated tests in setup-jest.spec.ts to verify configuration using setupZoneTestEnv.
  • Complete deletion of the deprecated setup-jest.js and setup-jest.mjs files.

Reviewed Changes

File Description
scripts/test-examples.js Removed legacy setup-jest asset names from the file copy operations.
src/config/setup-jest.spec.ts Removed tests for legacy setup-jest initialization and retained tests for setupZoneTestEnv.
setup-jest.mjs, setup-jest.js Deprecated setup files have been removed.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/config/setup-jest.spec.ts:109

  • [nitpick] The describe block title still references 'setup-jest' despite the switch to setupZoneTestEnv in tests. Consider updating the title to accurately reflect the new test environment setup for clarity.
describe('for ESM setup-jest, test environment initialization', () => {
BREAKING CHANGE

- Use `setupZoneTestEnv` as a replacement for zone test environment setup
- Use `setupZonelessTestEnv` as a replacement for zoneless test environment setup
@ahnpnl ahnpnl force-pushed the refactor/remove-setup-jest branch from 0260780 to bf1d6f9 Compare February 28, 2025 10:13
@ahnpnl ahnpnl requested a review from Copilot February 28, 2025 10:14

Choose a reason for hiding this comment

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

PR Overview

This PR removes deprecated setup-jest files and updates test configuration to use the new test environment setup utilities (setupZoneTestEnv and setupZonelessTestEnv).

  • Removed deprecated asset references and files for setup-jest.
  • Updated test specs to import and use setupZoneTestEnv for both CJS and ESM environments.

Reviewed Changes

File Description
scripts/test-examples.js Removed deprecated setup-jest file paths from the asset list.
src/config/setup-env.spec.ts Updated test descriptions and import statements to use setupZoneTestEnv.
setup-jest.mjs Removed the deprecated setup file.
setup-jest.js Removed the deprecated setup file.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/config/setup-env.spec.ts:66

  • Verify that removing the cleanup of globalThis.ngJest is intentional, as its persistence between tests could lead to state leakage.
delete globalThis.ngJest;

src/config/setup-env.spec.ts:111

  • Consider adding test coverage for the setupZonelessTestEnv function to ensure that both new test environments are properly validated.
describe('for ESM, test environment initialization', () => {
@ahnpnl ahnpnl marked this pull request as ready for review February 28, 2025 10:34
@ahnpnl ahnpnl merged commit 6bf89a4 into next Feb 28, 2025
10 checks passed
@ahnpnl ahnpnl deleted the refactor/remove-setup-jest branch February 28, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant