Merge pull request #1611 from github/aeisenberg/fix-flakes

aeisenberg · web-flow · commit a3fafc8e592b · 2022-10-25T08:26:26.000-07:00
Test cleanups
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -126,7 +126,7 @@ jobs:
         env:
           VSCODE_CODEQL_GITHUB_TOKEN: '${{ secrets.GITHUB_TOKEN }}'
         run: |
-          sudo apt-get install xvfb
+          unset DBUS_SESSION_BUS_ADDRESS
           /usr/bin/xvfb-run npm run integration
 
       - name: Run integration tests (Windows)
@@ -193,6 +193,7 @@ jobs:
         working-directory: extensions/ql-vscode
         if: matrix.os == 'ubuntu-latest'
         run: |
+          unset DBUS_SESSION_BUS_ADDRESS
           /usr/bin/xvfb-run npm run cli-integration
 
       - name: Run CLI tests (Windows)
diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts
@@ -1338,7 +1338,7 @@ export class CliVersionConstraint {
   /**
    * CLI version that supports the new query server.
    */
-  public static CLI_VERSION_WITH_NEW_QUERY_SERVER = new SemVer('2.11.0');
+  public static CLI_VERSION_WITH_NEW_QUERY_SERVER = new SemVer('2.11.1');
 
   constructor(private readonly cli: CodeQLCliServer) {
     /**/
@@ -1423,8 +1423,11 @@ export class CliVersionConstraint {
   async supportsNewQueryServer() {
     // TODO while under development, users _must_ opt-in to the new query server
     // by setting the `codeql.canaryQueryServer` setting to `true`.
-    // Ignore the version check for now.
-    return allowCanaryQueryServer();
-    // return this.isVersionAtLeast(CliVersionConstraint.CLI_VERSION_WITH_NEW_QUERY_SERVER);
+    return allowCanaryQueryServer() &&
+      this.isVersionAtLeast(CliVersionConstraint.CLI_VERSION_WITH_NEW_QUERY_SERVER);
+  }
+
+  async supportsNewQueryServerForTests() {
+    return this.isVersionAtLeast(CliVersionConstraint.CLI_VERSION_WITH_NEW_QUERY_SERVER);
   }
 }
diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts
@@ -10,13 +10,21 @@ export const ALL_SETTINGS: Setting[] = [];
 export class Setting {
   name: string;
   parent?: Setting;
+  private _hasChildren = false;
 
   constructor(name: string, parent?: Setting) {
     this.name = name;
     this.parent = parent;
+    if (parent !== undefined) {
+      parent._hasChildren = true;
+    }
     ALL_SETTINGS.push(this);
   }
 
+  get hasChildren() {
+    return this._hasChildren;
+  }
+
   get qualifiedName(): string {
     if (this.parent === undefined) {
       return this.name;
diff --git a/extensions/ql-vscode/src/helpers.ts b/extensions/ql-vscode/src/helpers.ts
@@ -22,7 +22,11 @@ fs.ensureDirSync(upgradesTmpDir);
 
 export const tmpDirDisposal = {
   dispose: () => {
-    tmpDir.removeCallback();
+    try {
+      tmpDir.removeCallback();
+    } catch (e) {
+      void logger.log(`Failed to remove temporary directory ${tmpDir.name}: ${e}`);
+    }
   }
 };
 
diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/global.helper.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/global.helper.ts
@@ -14,15 +14,6 @@ import { CUSTOM_CODEQL_PATH_SETTING } from '../../config';
 
 export const DB_URL = 'https://github.com/github/vscode-codeql/files/5586722/simple-db.zip';
 
-process.addListener('unhandledRejection', (reason) => {
-  if (reason instanceof Error && reason.message === 'Canceled') {
-    console.log('Cancellation requested after the test has ended.');
-    process.exit(0);
-  } else {
-    fail(String(reason));
-  }
-});
-
 // We need to resolve the path, but the final three segments won't exist until later, so we only resolve the
 // first portion of the path.
 export const dbLoc = path.join(fs.realpathSync(path.join(__dirname, '../../../')), 'build/tests/db.zip');
@@ -84,15 +75,25 @@ export default function(mocha: Mocha) {
       // This shuts down the extension and can only be run after all tests have completed.
       // If this is not called, then the test process will hang.
       if ('dispose' in extension) {
-        extension.dispose();
+        try {
+          extension.dispose();
+        } catch (e) {
+          console.warn('Failed to dispose extension', e);
+        }
       }
     }
   );
 
   // ensure temp directory is cleaned up.
   (mocha.options as any).globalTeardown.push(
     () => {
-      removeStorage?.();
+      try {
+        removeStorage?.();
+      } catch (e) {
+        // we are exiting anyway so don't worry about it.
+        // most likely the directory this is a test on Windows and some files are locked.
+        console.warn(`Failed to remove storage directory '${storagePath}': ${e}`);
+      }
     }
   );
 
diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/new-query.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/new-query.test.ts
@@ -16,7 +16,6 @@ import { QueryResultType } from '../../pure/new-messages';
 import { cleanDatabases, dbLoc, storagePath } from './global.helper';
 import { importArchiveDatabase } from '../../databaseFetcher';
 
-
 const baseDir = path.join(__dirname, '../../../test/data');
 
 const tmpDir = tmp.dirSync({ prefix: 'query_test_', keep: false, unsafeCleanup: true });
@@ -106,7 +105,7 @@ describe('using the new query server', function() {
         cliServer = extension.cliServer;
 
         cliServer.quiet = true;
-        if (!(await cliServer.cliConstraints.supportsNewQueryServer())) {
+        if (!(await cliServer.cliConstraints.supportsNewQueryServerForTests())) {
           this.ctx.skip();
         }
         qs = new QueryServerClient({
diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/run-remote-query.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/run-remote-query.test.ts
@@ -100,7 +100,6 @@ describe('Remote queries', function() {
       const querySubmissionResult = await runRemoteQuery(cli, credentials, fileUri, true, progress, cancellationTokenSource.token, variantAnalysisManager);
       expect(querySubmissionResult).to.be.ok;
       const queryPackRootDir = querySubmissionResult!.queryDirPath!;
-      printDirectoryContents(queryPackRootDir);
 
       // to retrieve the list of repositories
       expect(showQuickPickSpy).to.have.been.calledOnce;
@@ -113,7 +112,6 @@ describe('Remote queries', function() {
       expect(fs.readdirSync(queryPackRootDir).find(f => f.startsWith('qlpack-') && f.endsWith('-generated.tgz'))).not.to.be.undefined;
 
       const queryPackDir = path.join(queryPackRootDir, 'query-pack');
-      printDirectoryContents(queryPackDir);
 
       expect(fs.existsSync(path.join(queryPackDir, 'in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(queryPackDir, 'lib.qll'))).to.be.true;
@@ -128,7 +126,6 @@ describe('Remote queries', function() {
 
       // the compiled pack
       const compiledPackDir = path.join(queryPackDir, '.codeql/pack/codeql-remote/query/0.0.0/');
-      printDirectoryContents(compiledPackDir);
 
       expect(fs.existsSync(path.join(compiledPackDir, 'in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(compiledPackDir, 'lib.qll'))).to.be.true;
@@ -171,11 +168,9 @@ describe('Remote queries', function() {
       // check a few files that we know should exist and others that we know should not
 
       // the tarball to deliver to the server
-      printDirectoryContents(queryPackRootDir);
       expect(fs.readdirSync(queryPackRootDir).find(f => f.startsWith('qlpack-') && f.endsWith('-generated.tgz'))).not.to.be.undefined;
 
       const queryPackDir = path.join(queryPackRootDir, 'query-pack');
-      printDirectoryContents(queryPackDir);
 
       expect(fs.existsSync(path.join(queryPackDir, 'in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(queryPackDir, 'qlpack.yml'))).to.be.true;
@@ -189,7 +184,6 @@ describe('Remote queries', function() {
 
       // the compiled pack
       const compiledPackDir = path.join(queryPackDir, '.codeql/pack/codeql-remote/query/0.0.0/');
-      printDirectoryContents(compiledPackDir);
       expect(fs.existsSync(path.join(compiledPackDir, 'in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(compiledPackDir, 'qlpack.yml'))).to.be.true;
       verifyQlPack(path.join(compiledPackDir, 'qlpack.yml'), 'in-pack.ql', '0.0.0', await pathSerializationBroken());
@@ -208,7 +202,6 @@ describe('Remote queries', function() {
       expect(qlpackContents.dependencies?.['codeql/javascript-all']).to.equal('*');
 
       const libraryDir = path.join(compiledPackDir, '.codeql/libraries/codeql');
-      printDirectoryContents(libraryDir);
       const packNames = fs.readdirSync(libraryDir).sort();
 
       // check dependencies.
@@ -233,11 +226,9 @@ describe('Remote queries', function() {
       // check a few files that we know should exist and others that we know should not
 
       // the tarball to deliver to the server
-      printDirectoryContents(queryPackRootDir);
       expect(fs.readdirSync(queryPackRootDir).find(f => f.startsWith('qlpack-') && f.endsWith('-generated.tgz'))).not.to.be.undefined;
 
       const queryPackDir = path.join(queryPackRootDir, 'query-pack');
-      printDirectoryContents(queryPackDir);
 
       expect(fs.existsSync(path.join(queryPackDir, 'subfolder/in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(queryPackDir, 'qlpack.yml'))).to.be.true;
@@ -251,7 +242,6 @@ describe('Remote queries', function() {
 
       // the compiled pack
       const compiledPackDir = path.join(queryPackDir, '.codeql/pack/codeql-remote/query/0.0.0/');
-      printDirectoryContents(compiledPackDir);
       expect(fs.existsSync(path.join(compiledPackDir, 'otherfolder/lib.qll'))).to.be.true;
       expect(fs.existsSync(path.join(compiledPackDir, 'subfolder/in-pack.ql'))).to.be.true;
       expect(fs.existsSync(path.join(compiledPackDir, 'qlpack.yml'))).to.be.true;
@@ -270,7 +260,6 @@ describe('Remote queries', function() {
       expect(qlpackContents.dependencies?.['codeql/javascript-all']).to.equal('*');
 
       const libraryDir = path.join(compiledPackDir, '.codeql/libraries/codeql');
-      printDirectoryContents(libraryDir);
       const packNames = fs.readdirSync(libraryDir).sort();
 
       // check dependencies.
@@ -399,12 +388,4 @@ describe('Remote queries', function() {
   function getFile(file: string): Uri {
     return Uri.file(path.join(baseDir, file));
   }
-
-  function printDirectoryContents(dir: string) {
-    console.log(`DIR ${dir}`);
-    if (!fs.existsSync(dir)) {
-      console.log(`DIR ${dir} does not exist`);
-    }
-    fs.readdirSync(dir).sort().forEach(f => console.log(`  ${f}`));
-  }
 });
diff --git a/extensions/ql-vscode/src/vscode-tests/ensureCli.ts b/extensions/ql-vscode/src/vscode-tests/ensureCli.ts
@@ -30,15 +30,6 @@ import { workspace } from 'vscode';
  * exists. And the cli will not be re-downloaded if the zip already exists.
  */
 
-process.on('unhandledRejection', e => {
-  console.error('Unhandled rejection.');
-  console.error(e);
-  // Must use a setTimeout in order to ensure the log is fully flushed before exiting
-  setTimeout(() => {
-    process.exit(-1);
-  }, 2000);
-});
-
 const _1MB = 1024 * 1024;
 const _10MB = _1MB * 10;
 
diff --git a/extensions/ql-vscode/src/vscode-tests/index-template.ts b/extensions/ql-vscode/src/vscode-tests/index-template.ts
@@ -16,6 +16,15 @@ process.on('unhandledRejection', e => {
   }, 2000);
 });
 
+process.on('exit', code => {
+  // If the exit code is 7, then the test runner has completed, but
+  // there was an error in exiting vscode.
+  if (code === 7) {
+    console.warn('Attempted to exit with code 7. This is likely due to a failure to exit vscode. Ignoring this and exiting with code 0.');
+    process.exit(0);
+  }
+});
+
 /**
  * Helper function that runs all Mocha tests found in the
  * given test root directory.
diff --git a/extensions/ql-vscode/src/vscode-tests/minimal-workspace/index.ts b/extensions/ql-vscode/src/vscode-tests/minimal-workspace/index.ts
@@ -13,12 +13,3 @@ chai.use(sinonChai);
 export function run(): Promise<void> {
   return runTestsInDirectory(__dirname);
 }
-
-process.addListener('unhandledRejection', (reason) => {
-  if (reason instanceof Error && reason.message === 'Canceled') {
-    console.log('Cancellation requested after the test has ended.');
-    process.exit(0);
-  } else {
-    fail(String(reason));
-  }
-});
diff --git a/extensions/ql-vscode/src/vscode-tests/run-integration-tests.ts b/extensions/ql-vscode/src/vscode-tests/run-integration-tests.ts
@@ -13,7 +13,6 @@ import * as tmp from 'tmp-promise';
 // but we can be tricky and import directly from the out file.
 import { TestOptions } from 'vscode-test/out/runTest';
 
-
 // For CI purposes we want to leave this at 'stable' to catch any bugs
 // that might show up with new vscode versions released, even though
 // this makes testing not-quite-pure, but it can be changed for local
@@ -34,24 +33,21 @@ enum TestDir {
  * Run an integration test suite `suite`, retrying if it segfaults, at
  * most `tries` times.
  */
-async function runTestsWithRetryOnSegfault(suite: TestOptions, tries: number): Promise<void> {
+async function runTestsWithRetryOnSegfault(suite: TestOptions, tries: number): Promise<number> {
   for (let t = 0; t < tries; t++) {
     try {
       // Download and unzip VS Code if necessary, and run the integration test suite.
-      await runTests(suite);
-      return;
+      return await runTests(suite);
     } catch (err) {
       if (err === 'SIGSEGV') {
         console.error('Test runner segfaulted.');
         if (t < tries - 1)
           console.error('Retrying...');
-      }
-      else if (os.platform() === 'win32') {
+      } else if (os.platform() === 'win32') {
         console.error(`Test runner caught exception (${err})`);
         if (t < tries - 1)
           console.error('Retrying...');
-      }
-      else {
+      } else {
         throw err;
       }
     }
@@ -67,6 +63,7 @@ const tmpDir = tmp.dirSync({ unsafeCleanup: true });
  * See https://github.com/microsoft/vscode-test/blob/master/sample/test/runTest.ts
  */
 async function main() {
+  let exitCode = 0;
   try {
     const extensionDevelopmentPath = path.resolve(__dirname, '../..');
     const vscodeExecutablePath = await downloadAndUnzipVSCode(VSCODE_VERSION);
@@ -100,7 +97,7 @@ async function main() {
       const launchArgs = getLaunchArgs(dir as TestDir);
       console.log(`Next integration test dir: ${dir}`);
       console.log(`Launch args: ${launchArgs}`);
-      await runTestsWithRetryOnSegfault({
+      exitCode = await runTestsWithRetryOnSegfault({
         version: VSCODE_VERSION,
         vscodeExecutablePath,
         extensionDevelopmentPath,
@@ -111,8 +108,12 @@ async function main() {
     }
   } catch (err) {
     console.error(`Unexpected exception while running tests: ${err}`);
-    console.error((err as Error).stack);
-    process.exit(1);
+    if (err instanceof Error) {
+      console.error(err.stack);
+    }
+    exitCode = 1;
+  } finally {
+    process.exit(exitCode);
   }
 }
 
@@ -125,20 +126,23 @@ function getLaunchArgs(dir: TestDir) {
       return [
         '--disable-extensions',
         '--disable-gpu',
+        '--disable-workspace-trust',
         '--user-data-dir=' + path.join(tmpDir.name, dir, 'user-data')
       ];
 
     case TestDir.MinimalWorksspace:
       return [
         '--disable-extensions',
         '--disable-gpu',
+        '--disable-workspace-trust',
         '--user-data-dir=' + path.join(tmpDir.name, dir, 'user-data'),
         path.resolve(__dirname, '../../test/data')
       ];
 
     case TestDir.CliIntegration:
       // CLI integration tests requires a multi-root workspace so that the data and the QL sources are accessible.
       return [
+        '--disable-workspace-trust',
         '--disable-gpu',
         path.resolve(__dirname, '../../test/data'),
 
@@ -155,5 +159,4 @@ function getLaunchArgs(dir: TestDir) {
     default:
       assertNever(dir);
   }
-  return undefined;
 }
diff --git a/extensions/ql-vscode/src/vscode-tests/test-config.ts b/extensions/ql-vscode/src/vscode-tests/test-config.ts
