Skip to content

test(@angular/build): verify coverage ignore comments are preserved during compilation#32899

Open
clydin wants to merge 2 commits intoangular:mainfrom
clydin:vitest/coverage-tests
Open

test(@angular/build): verify coverage ignore comments are preserved during compilation#32899
clydin wants to merge 2 commits intoangular:mainfrom
clydin:vitest/coverage-tests

Conversation

@clydin
Copy link
Copy Markdown
Member

@clydin clydin commented Mar 30, 2026

The underlying Vitest coverage engine depends on specific developer comments like /* istanbul ignore next */ or /* v8 ignore next */ being present in the executing code to accurately isolate unmeasured blocks.

This commit adds strict behavioral tests to assert that the Angular CLI's in-memory compilation pipeline (via esbuild) properly preserves these structural comments and forwards them reliably to Vitest's coverage processing engine.

@clydin clydin force-pushed the vitest/coverage-tests branch 7 times, most recently from 6af4619 to bf91489 Compare April 1, 2026 21:22
When script optimization is disabled, set esbuild `legalComments` to 'inline'
to prevent it from moving ignore comments to the end of the file. This ensures
that coverage tools (like Istanbul and V8) can find the comments in place
and correctly associate them with the code they are supposed to ignore.
@clydin clydin force-pushed the vitest/coverage-tests branch 9 times, most recently from d75be32 to ca27cc0 Compare April 3, 2026 18:14
…uring compilation

The underlying Vitest coverage engine depends on specific developer comments like `/* istanbul ignore next */` or `/* v8 ignore next */` being present in the executing code to accurately isolate unmeasured blocks.

This commit adds strict behavioral tests to assert that the Angular CLI's in-memory compilation pipeline (via esbuild) properly preserves these structural comments and forwards them reliably to Vitest's coverage processing engine.
@clydin clydin force-pushed the vitest/coverage-tests branch from ca27cc0 to 9275953 Compare April 3, 2026 18:33
@clydin clydin marked this pull request as ready for review April 3, 2026 19:50
@clydin clydin added the target: patch This PR is targeted for the next patch release label Apr 3, 2026
Copy link
Copy Markdown

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

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 introduces new unit tests to verify that coverage ignore comments (istanbul and v8) are correctly respected during the build process. Additionally, it updates the esbuild configuration to set legalComments to 'inline' when optimization is disabled, ensuring license comments are preserved in the output. I have suggested refactoring the repetitive coverage assertion logic in the new test file into a helper function to improve maintainability.

Comment on lines +89 to +103
const coverageMap = JSON.parse(harness.readFile('coverage/test/coverage-final.json'));
const appComponentPath = Object.keys(coverageMap).find((p) =>
p.includes('app.component.ts'),
);
expect(appComponentPath).toBeDefined();

const appComponentCoverage = coverageMap[appComponentPath!];

const statementCounts = Object.values(appComponentCoverage.s) as number[];
const hasUncoveredStatements = statementCounts.some((count) => count === 0);
expect(hasUncoveredStatements)
.withContext(
'There should be no uncovered statements as the uncalled function was ignored',
)
.toBeFalse();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic for parsing the coverage report and asserting that no statements are uncovered is duplicated across all test cases in this file. Extracting this into a helper function would improve maintainability and make the tests more concise.

For example, you could define a helper like this inside the describeBuilder block:

async function assertNoUncoveredStatements(contextMessage: string) {
  harness.expectFile('coverage/test/coverage-final.json').toExist();
  const coverageMap = JSON.parse(harness.readFile('coverage/test/coverage-final.json'));
  const appComponentPath = Object.keys(coverageMap).find((p) => p.includes('app.component.ts'));
  expect(appComponentPath).toBeDefined();

  const appComponentCoverage = coverageMap[appComponentPath!];
  const statementCounts = Object.values(appComponentCoverage.s) as number[];
  expect(statementCounts.some((count) => count === 0))
    .withContext(contextMessage)
    .toBeFalse();
}

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

Labels

area: @angular/build target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant