test(localize): ensure tests pass on Windows (#40952)

These tests were relying upon unix-like paths, which caused them to fail on Windows. Note that the `filegroup` Bazel rule tends not to work well on Windows, so this has been replaced with `copy_to_bin` instead. PR Close #40952
angular · Feb 24, 2021 · 51a7977 · 51a7977
1 parent 5ae28a4
commit 51a7977
Show file tree

Hide file tree

Showing 25 changed files with 207 additions and 130 deletions.
diff --git a/packages/localize/src/tools/src/extract/source_files/es5_extract_plugin.ts b/packages/localize/src/tools/src/extract/source_files/es5_extract_plugin.ts
@@ -35,7 +35,7 @@ export function makeEs5ExtractPlugin(
             // If we get a BabelParseError here then something went wrong with Babel itself
             // since there must be something wrong with the structure of the AST generated
             // by Babel parsing a TaggedTemplateExpression.
-            throw buildCodeFrameError(callPath, e);
+            throw buildCodeFrameError(fs, callPath, e);
           } else {
             throw e;
           }

diff --git a/packages/localize/src/tools/src/source_file_utils.ts b/packages/localize/src/tools/src/source_file_utils.ts
@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {AbsoluteFsPath, getFileSystem, PathManipulation} from '@angular/compiler-cli/src/ngtsc/file_system';
+import {absoluteFrom, AbsoluteFsPath, getFileSystem, PathManipulation} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {ɵisMissingTranslationError, ɵmakeTemplateObject, ɵParsedTranslation, ɵSourceLocation, ɵtranslate} from '@angular/localize';
 import {NodePath} from '@babel/traverse';
 import * as t from '@babel/types';
@@ -400,8 +400,19 @@ export function isBabelParseError(e: any): e is BabelParseError {
   return e.type === 'BabelParseError';
 }
 
-export function buildCodeFrameError(path: NodePath, e: BabelParseError): string {
-  const filename = path.hub.file.opts.filename || '(unknown file)';
+export function buildCodeFrameError(
+    fs: PathManipulation, path: NodePath, e: BabelParseError): string {
+  let filename = path.hub.file.opts.filename;
+  if (filename) {
+    filename = fs.resolve(filename);
+    let cwd = path.hub.file.opts.cwd;
+    if (cwd) {
+      cwd = fs.resolve(cwd);
+      filename = fs.relative(cwd, filename);
+    }
+  } else {
+    filename = '(unknown file)';
+  }
   const message = path.hub.file.buildCodeFrameError(e.node, e.message).message;
   return `${filename}: ${message}`;
 }
@@ -434,9 +445,14 @@ export function serializeLocationPosition(location: ɵSourceLocation): string {
 
 function getFileFromPath(fs: PathManipulation, path: NodePath|undefined): AbsoluteFsPath|null {
   const opts = path?.hub.file.opts;
-  return opts?.filename ?
-      fs.resolve(opts.generatorOpts.sourceRoot ?? opts.cwd, fs.relative(opts.cwd, opts.filename)) :
-      null;
+  const filename = opts?.filename;
+  if (!filename) {
+    return null;
+  }
+  const relativePath = fs.relative(opts.cwd, filename);
+  const root = opts.generatorOpts.sourceRoot ?? opts.cwd;
+  const absPath = fs.resolve(root, relativePath);
+  return absPath;
 }
 
 function getLineAndColumn(loc: {line: number, column: number}): {line: number, column: number} {

diff --git a/packages/localize/src/tools/src/translate/main.ts b/packages/localize/src/tools/src/translate/main.ts
@@ -105,7 +105,7 @@ if (require.main === module) {
   const sourceRootPath = options.r;
   const sourceFilePaths = glob.sync(options.s, {cwd: sourceRootPath, nodir: true});
   const translationFilePaths: (string|string[])[] = convertArraysFromArgs(options.t);
-  const outputPathFn = getOutputPathFn(fs.resolve(options.o));
+  const outputPathFn = getOutputPathFn(fs, fs.resolve(options.o));
   const diagnostics = new Diagnostics();
   const missingTranslation = options.m as DiagnosticHandlingStrategy;
   const duplicateTranslation = options.d as DiagnosticHandlingStrategy;

diff --git a/packages/localize/src/tools/src/translate/output_path.ts b/packages/localize/src/tools/src/translate/output_path.ts
@@ -5,8 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {join} from 'path';
+import {AbsoluteFsPath, PathManipulation} from '@angular/compiler-cli/src/ngtsc/file_system';
 
 /**
  * A function that will return an absolute path to where a file is to be written, given a locale and
@@ -23,8 +22,8 @@ export interface OutputPathFn {
  * The special `{{LOCALE}}` marker will be replaced with the locale code of the current translation.
  * @param outputFolder An absolute path to the folder containing this set of translations.
  */
-export function getOutputPathFn(outputFolder: AbsoluteFsPath): OutputPathFn {
+export function getOutputPathFn(fs: PathManipulation, outputFolder: AbsoluteFsPath): OutputPathFn {
   const [pre, post] = outputFolder.split('{{LOCALE}}');
-  return post === undefined ? (_locale, relativePath) => join(pre, relativePath) :
-                              (locale, relativePath) => join(pre + locale + post, relativePath);
+  return post === undefined ? (_locale, relativePath) => fs.join(pre, relativePath) :
+                              (locale, relativePath) => fs.join(pre + locale + post, relativePath);
 }
diff --git a/packages/localize/src/tools/src/translate/source_files/es2015_translate_plugin.ts b/packages/localize/src/tools/src/translate/source_files/es2015_translate_plugin.ts
@@ -41,7 +41,7 @@ export function makeEs2015TranslatePlugin(
             // If we get a BabelParseError here then something went wrong with Babel itself
             // since there must be something wrong with the structure of the AST generated
             // by Babel parsing a TaggedTemplateExpression.
-            throw buildCodeFrameError(path, e);
+            throw buildCodeFrameError(fs, path, e);
           } else {
             throw e;
           }

diff --git a/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts b/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts
@@ -37,7 +37,7 @@ export function makeEs5TranslatePlugin(
           }
         } catch (e) {
           if (isBabelParseError(e)) {
-            diagnostics.error(buildCodeFrameError(callPath, e));
+            diagnostics.error(buildCodeFrameError(fs, callPath, e));
           } else {
             throw e;
           }

diff --git a/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts b/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts
@@ -81,6 +81,7 @@ export class SourceFileTranslationHandler implements TranslationHandler {
         makeEs2015TranslatePlugin(diagnostics, translationBundle.translations, options, this.fs),
         makeEs5TranslatePlugin(diagnostics, translationBundle.translations, options, this.fs),
       ],
+      cwd: sourceRoot,
       filename,
     });
     if (translated && translated.code) {

diff --git a/packages/localize/src/tools/test/BUILD.bazel b/packages/localize/src/tools/test/BUILD.bazel
@@ -15,6 +15,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/logging/testing",
         "//packages/localize",
         "//packages/localize/src/tools",
+        "//packages/localize/src/tools/test/helpers",
         "//packages/localize/src/utils",
         "@npm//@babel/core",
         "@npm//@babel/generator",

diff --git a/packages/localize/src/tools/test/extract/extractor_spec.ts b/packages/localize/src/tools/test/extract/extractor_spec.ts
@@ -5,13 +5,13 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {absoluteFrom, getFileSystem, relativeFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
+import {absoluteFrom, getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {MockLogger} from '@angular/compiler-cli/src/ngtsc/logging/testing';
 
 import {MessageExtractor} from '../../src/extract/extraction';
+import {runInNativeFileSystem} from '../helpers';
 
-runInEachFileSystem(() => {
+runInNativeFileSystem(() => {
   describe('extractMessages', () => {
     it('should extract a message for each $localize template tag', () => {
       const fs = getFileSystem();
@@ -20,7 +20,7 @@ runInEachFileSystem(() => {
       const filename = 'relative/path.js';
       const file = fs.resolve(basePath, filename);
       const extractor = new MessageExtractor(fs, logger, {basePath});
-      fs.ensureDir(absoluteFrom('/root/path/relative'));
+      fs.ensureDir(fs.dirname(file));
       fs.writeFile(file, [
         '$localize`:meaning|description:a${1}b${2}c`;',
         '$localize(__makeTemplateObject(["a", ":custom-placeholder:b", "c"], ["a", ":custom-placeholder:b", "c"]), 1, 2);',
@@ -40,38 +40,28 @@ runInEachFileSystem(() => {
           {
             start: {line: 0, column: 10},
             end: {line: 0, column: 32},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: ':meaning|description:a',
           },
           {
             start: {line: 0, column: 36},
             end: {line: 0, column: 37},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: 'b',
           },
           {
             start: {line: 0, column: 41},
             end: {line: 0, column: 42},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: 'c',
           }
         ],
         text: 'a{$PH}b{$PH_1}c',
         placeholderNames: ['PH', 'PH_1'],
         substitutions: jasmine.any(Object),
         substitutionLocations: {
-          PH: {
-            start: {line: 0, column: 34},
-            end: {line: 0, column: 35},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '1'
-          },
-          PH_1: {
-            start: {line: 0, column: 39},
-            end: {line: 0, column: 40},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '2'
-          }
+          PH: {start: {line: 0, column: 34}, end: {line: 0, column: 35}, file, text: '1'},
+          PH_1: {start: {line: 0, column: 39}, end: {line: 0, column: 40}, file, text: '2'}
         },
         legacyIds: [],
         location: {
@@ -92,38 +82,29 @@ runInEachFileSystem(() => {
           {
             start: {line: 1, column: 69},
             end: {line: 1, column: 72},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: '"a"',
           },
           {
             start: {line: 1, column: 74},
             end: {line: 1, column: 97},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: '":custom-placeholder:b"',
           },
           {
             start: {line: 1, column: 99},
             end: {line: 1, column: 102},
-            file: absoluteFrom('/root/path/relative/path.js'),
+            file,
             text: '"c"',
           }
         ],
         text: 'a{$custom-placeholder}b{$PH_1}c',
         placeholderNames: ['custom-placeholder', 'PH_1'],
         substitutions: jasmine.any(Object),
         substitutionLocations: {
-          'custom-placeholder': {
-            start: {line: 1, column: 106},
-            end: {line: 1, column: 107},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '1'
-          },
-          PH_1: {
-            start: {line: 1, column: 109},
-            end: {line: 1, column: 110},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '2'
-          }
+          'custom-placeholder':
+              {start: {line: 1, column: 106}, end: {line: 1, column: 107}, file, text: '1'},
+          PH_1: {start: {line: 1, column: 109}, end: {line: 1, column: 110}, file, text: '2'}
         },
         legacyIds: [],
         location: {
@@ -145,38 +126,13 @@ runInEachFileSystem(() => {
         placeholderNames: ['PH', 'PH_1'],
         substitutions: jasmine.any(Object),
         substitutionLocations: {
-          PH: {
-            start: {line: 2, column: 26},
-            end: {line: 2, column: 27},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '1'
-          },
-          PH_1: {
-            start: {line: 2, column: 31},
-            end: {line: 2, column: 32},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: '2'
-          }
+          PH: {start: {line: 2, column: 26}, end: {line: 2, column: 27}, file, text: '1'},
+          PH_1: {start: {line: 2, column: 31}, end: {line: 2, column: 32}, file, text: '2'}
         },
         messagePartLocations: [
-          {
-            start: {line: 2, column: 10},
-            end: {line: 2, column: 24},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: ':@@custom-id:a'
-          },
-          {
-            start: {line: 2, column: 28},
-            end: {line: 2, column: 29},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: 'b'
-          },
-          {
-            start: {line: 2, column: 33},
-            end: {line: 2, column: 34},
-            file: absoluteFrom('/root/path/relative/path.js'),
-            text: 'c'
-          }
+          {start: {line: 2, column: 10}, end: {line: 2, column: 24}, file, text: ':@@custom-id:a'},
+          {start: {line: 2, column: 28}, end: {line: 2, column: 29}, file, text: 'b'},
+          {start: {line: 2, column: 33}, end: {line: 2, column: 34}, file, text: 'c'}
         ],
         legacyIds: [],
         location: {

diff --git a/packages/localize/src/tools/test/extract/integration/BUILD.bazel b/packages/localize/src/tools/test/extract/integration/BUILD.bazel
@@ -15,6 +15,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/testing",
         "//packages/localize/src/tools",
         "//packages/localize/src/tools/test:test_lib",
+        "//packages/localize/src/tools/test/helpers",
     ],
 )
 

diff --git a/packages/localize/src/tools/test/extract/integration/main_spec.ts b/packages/localize/src/tools/test/extract/integration/main_spec.ts
@@ -7,15 +7,15 @@
  */
 import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, setFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {InvalidFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/src/invalid_file_system';
-import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
 import {MockLogger} from '@angular/compiler-cli/src/ngtsc/logging/testing';
 import {loadTestDirectory} from '@angular/compiler-cli/src/ngtsc/testing';
 
 import {extractTranslations} from '../../../src/extract/main';
 import {FormatOptions} from '../../../src/extract/translation_files/format_options';
+import {runInNativeFileSystem} from '../../helpers';
 import {toAttributes} from '../translation_files/utils';
 
-runInEachFileSystem(() => {
+runInNativeFileSystem(() => {
   let fs: FileSystem;
   let logger: MockLogger;
   let rootPath: AbsoluteFsPath;

diff --git a/packages/localize/src/tools/test/extract/integration/test_files/BUILD.bazel b/packages/localize/src/tools/test/extract/integration/test_files/BUILD.bazel
@@ -1,6 +1,7 @@
 package(default_visibility = ["//packages/localize/src/tools/test/extract/integration:__pkg__"])
 
 load("@npm//typescript:index.bzl", "tsc")
+load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
 
 tsc(
     name = "compile_es5",
@@ -44,7 +45,8 @@ tsc(
     data = glob(["src/*.ts"]),
 )
 
-filegroup(
+# Use copy_to_bin since filegroup doesn't seem to work on Windows.
+copy_to_bin(
     name = "test_files",
     srcs = glob([
         "**/*.js",

diff --git a/packages/localize/src/tools/test/extract/source_files/es5_extract_plugin_spec.ts b/packages/localize/src/tools/test/extract/source_files/es5_extract_plugin_spec.ts
@@ -6,30 +6,42 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
+import {FileSystem, getFileSystem, PathSegment, relativeFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {ɵParsedMessage} from '@angular/localize/private';
 import {transformSync} from '@babel/core';
 
 import {makeEs5ExtractPlugin} from '../../../src/extract/source_files/es5_extract_plugin';
+import {runInNativeFileSystem} from '../../helpers';
+
+runInNativeFileSystem(() => {
+  let fs: FileSystem;
+  let testPath: PathSegment;
+
+  beforeEach(() => {
+    fs = getFileSystem();
+    testPath = relativeFrom('app/dist/test.js');
+  });
 
-runInEachFileSystem(() => {
   describe('makeEs5ExtractPlugin()', () => {
     it('should error with code-frame information if the first argument to `$localize` is not an array',
        () => {
          const input = '$localize(null, [])';
          expect(() => transformCode(input))
              .toThrowError(
-                 'Cannot create property \'message\' on string \'/app/dist/test.js: Unexpected messageParts for `$localize` (expected an array of strings).\n' +
+                 `Cannot create property 'message' on string '${testPath}: ` +
+                 'Unexpected messageParts for `$localize` (expected an array of strings).\n' +
                  '> 1 | $localize(null, [])\n' +
                  '    |           ^^^^\'');
        });
 
     function transformCode(input: string): ɵParsedMessage[] {
       const messages: ɵParsedMessage[] = [];
+      const cwd = fs.resolve('/');
+      const filename = fs.resolve(cwd, testPath);
       transformSync(input, {
         plugins: [makeEs5ExtractPlugin(getFileSystem(), messages)],
-        filename: '/app/dist/test.js'
+        filename,
+        cwd,
       })!.code!;
       return messages;
     }

diff --git a/packages/localize/src/tools/test/helpers/BUILD.bazel b/packages/localize/src/tools/test/helpers/BUILD.bazel
@@ -0,0 +1,14 @@
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "helpers",
+    testonly = True,
+    srcs = glob(
+        ["**/*.ts"],
+    ),
+    visibility = ["//packages/localize/src/tools/test:__subpackages__"],
+    deps = [
+        "//packages/compiler-cli/src/ngtsc/file_system",
+        "//packages/compiler-cli/src/ngtsc/file_system/testing",
+    ],
+)