Skip to content

Commit

Permalink
build: re-enable linker compliance tests (angular#53571)
Browse files Browse the repository at this point in the history
The linker compliance tests were disabled with a Babel update and
nobody realized for quite a while, via
angular#49914.

As we've came across this lost coverage, which also is quite
impactful as all libraries depend on linked output- I've took initiative
to debug the root cause as there was no follow-up.
angular#51647.

It turned out to be a highly complex issue that is non-trivial to fix,
but at least we should try to resurrect the significant portion of test
coverage by still running the linker tests- avoiding regressions, or
unexpected issues (like with defer being developed). We can work on
re-enabling and fixing source-maps separately.

Tracked via angular#51647.

PR Close angular#53571
  • Loading branch information
devversion authored and danieljancar committed Jan 26, 2024
1 parent 29192d1 commit ed59148
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 15 deletions.
4 changes: 0 additions & 4 deletions packages/compiler-cli/test/compliance/linked/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ jasmine_node_test(
"//packages/compiler-cli/test/compliance/test_cases",
],
shard_count = 2,
tags = [
# TODO: renable this test after debugging the issues related to babel updates
"manual",
],
deps = [
":test_lib",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import {ComplianceTest} from '../test_helpers/get_compliance_tests';
import {parseGoldenPartial} from '../test_helpers/golden_partials';
import {runTests} from '../test_helpers/test_runner';

runTests('linked compile', linkPartials);
runTests('linked compile', linkPartials, {
// TODO: Remove when https://github.com/angular/angular/issues/51647 is resolved.
skipMappingChecks: true,
});

/**
* Link all the partials specified in the given `test`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {replaceMacros} from './expected_file_macros';
import {verifyUniqueFunctions} from './function_checks';
import {ExpectedFile, ExtraCheck} from './get_compliance_tests';
import {verifyPlaceholdersIntegrity, verifyUniqueConsts} from './i18n_checks';
import {checkMappings} from './sourcemap_helpers';
import {stripAndCheckMappings} from './sourcemap_helpers';

type ExtraCheckFunction = (generated: string, ...extraArgs: any[]) => boolean;
const EXTRA_CHECK_FUNCTIONS: Record<string, ExtraCheckFunction> = {
Expand All @@ -31,10 +31,13 @@ const EXTRA_CHECK_FUNCTIONS: Record<string, ExtraCheckFunction> = {
* @param testPath Path to the current test case (relative to the basePath).
* @param failureMessage The message to display if the expectation fails.
* @param expectedFiles The list of expected-generated pairs to compare.
* @param skipMappingCheck Whether to skip checking source mappings.
* TODO: Remove this option. This only exists until we fix:
* https://github.com/angular/angular/issues/51647.
*/
export function checkExpectations(
fs: ReadonlyFileSystem, testPath: string, failureMessage: string, expectedFiles: ExpectedFile[],
extraChecks: ExtraCheck[]): void {
extraChecks: ExtraCheck[], skipMappingCheck = false): void {
const builtDirectory = getBuildOutputDirectory(fs);
for (const expectedFile of expectedFiles) {
const expectedPath = fs.resolve(getRootDirectory(fs), expectedFile.expected);
Expand All @@ -58,7 +61,9 @@ export function checkExpectations(
const generated = fs.readFile(generatedPath);
let expected = fs.readFile(expectedPath);
expected = replaceMacros(expected);
expected = checkMappings(fs, generated, generatedPath, expected, expectedPath);
expected = stripAndCheckMappings(
fs, generated, generatedPath, expected, expectedPath,
/** skipMappingCheck */ !!skipMappingCheck);

expectEmit(
generated, expected,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,19 @@ import {SourceFileLoader} from '../../../src/ngtsc/sourcemaps';
* @param expectedSource The content of the expected source file, containing mapping information.
* @returns The content of the expected source file, stripped of the mapping information.
*/
export function checkMappings(
export function stripAndCheckMappings(
fs: ReadonlyFileSystem, generated: string, generatedPath: AbsoluteFsPath,
expectedSource: string, expectedPath: AbsoluteFsPath): string {
expectedSource: string, expectedPath: AbsoluteFsPath, skipMappingCheck: boolean): string {
// Generate the candidate source maps.
const actualMappings = getMappedSegments(fs, generatedPath, generated);

const {expected, mappings} = extractMappings(fs, expectedSource);

// TODO: Remove when https://github.com/angular/angular/issues/51647 is fixed.
if (skipMappingCheck) {
return expected;
}

const failures: string[] = [];
for (const expectedMapping of mappings) {
const failure = checkMapping(actualMappings, expectedMapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ function getFilenameForLocalCompilation(fileName: string): string {
*/
export function runTests(
type: CompilationMode, compileFn: (fs: FileSystem, test: ComplianceTest) => CompileResult,
options = {
isLocalCompilation: false
}) {
options: {isLocalCompilation?: boolean, skipMappingChecks?: boolean} = {}) {
describe(`compliance tests (${type})`, () => {
for (const test of getAllComplianceTests()) {
if (!test.compilationModeFilter.includes(type)) {
Expand All @@ -73,7 +71,7 @@ export function runTests(
const fs = initMockTestFileSystem(test.realTestPath);
const {errors} = compileFn(fs, test);
for (const expectation of test.expectations) {
transformExpectation(expectation, options.isLocalCompilation);
transformExpectation(expectation, !!options.isLocalCompilation);
if (expectation.expectedErrors.length > 0) {
checkErrors(
test.relativePath, expectation.failureMessage, expectation.expectedErrors,
Expand All @@ -82,7 +80,7 @@ export function runTests(
checkNoUnexpectedErrors(test.relativePath, errors);
checkExpectations(
fs, test.relativePath, expectation.failureMessage, expectation.files,
expectation.extraChecks);
expectation.extraChecks, options.skipMappingChecks);
}
}
});
Expand Down

0 comments on commit ed59148

Please sign in to comment.