Repository: kythe/kythe
Pull request #4944 (2021-05-22 00:08:09)

Refactor TS indexer unit tests.
1. Introduce TestCase class. This improves code clarity. And helps to prevents issues like currently
where we for each group of files in a directory (single test case) we run tests N times, where N -
number of files in that directory instead of running test one.

2. Pass pathVNames to indexer. This is currently doesn't affect test run but it provides more
complete test setup.

nbeloglazov: @blickly to take look for Typescript
shahms: /gcbrun
shahms: /gcbrun
shahms: /gcbrun

Patch:
diff --git a/kythe/typescript/indexer.ts b/kythe/typescript/indexer.ts
index d1f4ed8ba..85c429cc8 100644
--- a/kythe/typescript/indexer.ts
+++ b/kythe/typescript/indexer.ts
@@ -637,6 +637,17 @@ class StandardIndexerContext implements IndexerHost {
 
   /**
    * pathToVName returns the VName for a given file path.
+   *
+   * This function is used for 2 distinct cases that should be ideally separated
+   * in 2 different functions. `path` can be one of two:
+   * 1. Full path like 'bazel-out/genfiles/path/to/file.ts'.
+   *    This path is used to build VNames for files and anchors.
+   * 2. Module name like 'path/to/file'.
+   *    This path is used to build VNames for semantic nodes.
+   *
+   * Only for full paths `pathVnames` contains an entry. For short paths (module
+   * names) this function will defaults to calculating vname based on path
+   * and compilation unit.
    */
   pathToVName(path: string): VName {
     const vname = this.pathVNames.get(path);


Test patch:
diff --git a/kythe/typescript/test.ts b/kythe/typescript/test.ts
index ad46ab68e..7198f6708 100644
--- a/kythe/typescript/test.ts
+++ b/kythe/typescript/test.ts
@@ -33,7 +33,9 @@ import * as indexer from './indexer';
 import * as kythe from './kythe';
 
 const KYTHE_PATH = process.env['KYTHE'] || '/opt/kythe';
-const RUNFILES = process.env['RUNFILES_DIR'];
+const RUNFILES = process.env['TEST_SRCDIR'];
+
+
 
 const ENTRYSTREAM = RUNFILES ?
     path.resolve('kythe/go/platform/tools/entrystream/entrystream') :
@@ -41,6 +43,18 @@ const ENTRYSTREAM = RUNFILES ?
 const VERIFIER = RUNFILES ? path.resolve('kythe/cxx/verifier/verifier') :
                             path.resolve(KYTHE_PATH, 'tools/verifier');
 
+/** Record representing a single test case to run. */
+interface TestCase {
+  /** Name of the test case. Used for reporting. */
+  readonly name: string;
+  /**
+   * List of files used for indexing. All these files passed together as source
+   * files to the indexer.
+   */
+  readonly files: string[];
+}
+
+
 /**
  * createTestCompilerHost creates a ts.CompilerHost that caches the default
  * libraries.  This prevents re-parsing the (big) TypeScript standard library
@@ -73,7 +87,7 @@ function createTestCompilerHost(options: ts.CompilerOptions): ts.CompilerHost {
  * be run async; if there's an error, it will reject the promise.
  */
 function verify(
-    host: ts.CompilerHost, options: ts.CompilerOptions, testFiles: string[],
+    host: ts.CompilerHost, options: ts.CompilerOptions, testCase: TestCase,
     plugins?: indexer.Plugin[]): Promise<void> {
   const compilationUnit: kythe.VName = {
     corpus: 'testcorpus',
@@ -82,6 +96,7 @@ function verify(
     signature: '',
     language: '',
   };
+  const testFiles = testCase.files;
   const program = ts.createProgram(testFiles, options, host);
 
   const verifier = child_process.spawn(
@@ -91,11 +106,16 @@ function verify(
         stdio: ['pipe', process.stdout, process.stderr],
         shell: true,
       });
+  const pathVnames = new Map<string, kythe.VName>();
+  for (const file of testFiles) {
+    pathVnames.set(file, {...compilationUnit, path: file});
+  }
 
   try {
-    indexer.index(compilationUnit, new Map(), testFiles, program, (obj: {}) => {
-      verifier.stdin.write(JSON.stringify(obj) + '\n');
-    }, plugins);
+    indexer.index(
+        compilationUnit, pathVnames, testFiles, program, (obj: {}) => {
+          verifier.stdin.write(JSON.stringify(obj) + '\n');
+        }, plugins);
   } finally {
     // Ensure we close stdin on the verifier even on crashes, or otherwise
     // we hang waiting for the verifier to complete.
@@ -142,20 +162,26 @@ function collectTSFilesInDirectoryRecursively(dir: string, result: string[]) {
  * 3. All individual files in subfolders not ending with '_group' will be
  *    returned as group of one file. So they are tested separately.
  */
-function getTestFileGroups(dir: string): string[][] {
+function getTestCases(options: ts.CompilerOptions, dir: string): TestCase[] {
   const result = [];
   for (const file of fs.readdirSync(dir, {withFileTypes: true})) {
     const relativeName = `${dir}/${file.name}`;
     if (file.isDirectory()) {
       if (file.name.endsWith('_group')) {
-        const group: string[] = [];
-        collectTSFilesInDirectoryRecursively(relativeName, group);
-        result.push(group);
+        const files: string[] = [];
+        collectTSFilesInDirectoryRecursively(relativeName, files);
+        result.push({
+          name: relativeName,
+          files,
+        });
       } else {
-        result.push(...getTestFileGroups(`${dir}/${file.name}`));
+        result.push(...getTestCases(options, relativeName));
       }
     } else if (file.name.endsWith('.ts')) {
-      result.push([path.resolve(relativeName)]);
+      result.push({
+        name: relativeName,
+        files: [path.resolve(relativeName)],
+      });
     }
   }
   return result;
@@ -166,10 +192,9 @@ function getTestFileGroups(dir: string): string[][] {
  * groups that contain at least one file that matches at least one filter.
  * Matching is done by simply checking for substring.
  */
-function filterTestFileGroups(
-    testFileGroups: string[][], filters: string[]): string[][] {
-  return testFileGroups.filter(fileGroup => {
-    for (const file of fileGroup) {
+function filterTestCases(testCases: TestCase[], filters: string[]): TestCase[] {
+  return testCases.filter(testCase => {
+    for (const file of testCase.files) {
       if (filters.some(filter => file.includes(filter))) {
         return true;
       }
@@ -181,30 +206,27 @@ function filterTestFileGroups(
 async function testIndexer(filters: string[], plugins?: indexer.Plugin[]) {
   const config =
       indexer.loadTsConfig('testdata/tsconfig.for.tests.json', 'testdata');
-  let testFilesGroups = getTestFileGroups('testdata');
+  let testCases = getTestCases(config.options, 'testdata');
   if (filters.length !== 0) {
-    testFilesGroups = filterTestFileGroups(testFilesGroups, filters);
+    testCases = filterTestCases(testCases, filters);
   }
 
   const host = createTestCompilerHost(config.options);
-  for (const testFiles of testFilesGroups) {
-    for (const testFile of testFiles) {
-      const testName = path.relative(config.options.rootDir!, testFile);
-      if (testName.endsWith('plugin.ts')) {
-        // plugin.ts is tested by testPlugin() test.
-        continue;
-      }
-      const start = new Date().valueOf();
-      process.stdout.write(`${testName}: `);
-      try {
-        await verify(host, config.options, testFiles, plugins);
-      } catch (e) {
-        console.log('FAIL');
-        throw e;
-      }
-      const time = new Date().valueOf() - start;
-      console.log('PASS', `${time}ms`);
+  for (const testCase of testCases) {
+    if (testCase.name.endsWith('plugin.ts')) {
+      // plugin.ts is tested by testPlugin() test.
+      continue;
+    }
+    const start = new Date().valueOf();
+    process.stdout.write(`${testCase.name}: `);
+    try {
+      await verify(host, config.options, testCase, plugins);
+    } catch (e) {
+      console.log('FAIL');
+      throw e;
     }
+    const time = new Date().valueOf() - start;
+    console.log('PASS', `${time}ms`);
   }
   return 0;
 }