Revert "regression-tests: update custom mocha reporter to exit rather… (#41542)

Revert "regression-tests: update custom mocha reporter to exit rather than mocha itself (#41266)"

This reverts commit 3b31a19dcb.

This change's build's QA test timed out (https://buildkite.com/sourcegraph/sourcegraph/builds/172014#01831e2c-78c0-4046-8270-b3a8f5d48a90), the previous build aa2c9830f1 did not. The QA build hanging is causing all builds in main to fail after timing out.

.mocharc.js

@@ -1,15 +1,11 @@
-const { execSync } = require('child_process')
-
-const repoRoot = execSync('git rev-parse --show-toplevel').toString().trimEnd()
-
 module.exports = {
   require: [
     'ts-node/register/transpile-only',
     'abort-controller/polyfill',
-    repoRoot + '/client/shared/dev/fetch',
-    repoRoot + '/client/shared/dev/suppressPollyErrors',
+    __dirname + '/client/shared/dev/fetch',
+    __dirname + '/client/shared/dev/suppressPollyErrors',
   ],
-  reporter: repoRoot + '/client/shared/dev/customMochaSpecReporter.js',
+  reporter: __dirname + '/client/shared/dev/customMochaSpecReporter.js',
   extension: ['js', 'ts'],
   // 1 minute test timeout. This must be greater than the default Puppeteer
   // command timeout of 30s in order to get the stack trace to point to the

client/shared/dev/customMochaSpecReporter.js

@@ -1,8 +1,6 @@
-const { execSync } = require('child_process')
 const { Console } = require('console')
 const fs = require('fs')
 
-const repoRoot = execSync('git rev-parse --show-toplevel').toString().trimEnd()
 const mocha = require('mocha')
 
 const originalConsoleLog = mocha.reporters.Base.consoleLog
@@ -22,6 +20,7 @@ class SpecFileReporter extends mocha.reporters.Spec {
   constructor(runner, options) {
     super(runner, options)
     this.title = 'placeholder'
+    this.buildkite = false
 
     if ('BUILDKITE' in process.env) {
       this.buildkite = true
@@ -30,26 +29,16 @@ class SpecFileReporter extends mocha.reporters.Spec {
     }
 
     if ('BUILDKITE_LABEL' in process.env) {
-      this.title = process.env.BUILDKITE_LABEL || 'placeholder'
+      this.title = process.env.BUILDKIATE_LABEL
     }
 
     if (this.buildkite === true && typeof process.env.BUILDKITE_LABEL === undefined) {
-      console.info(`In Buildkite but BUILDKITE_LABEL not found in environment. Using title '${this.title}'`)
+      console.warn(
+        `In Buildkite but BUILDKITE_LABEL not found in environment. Using title '${this.title || 'placeholder'}'`
+      )
     }
   }
 
-  safeClose(stream) {
-    return new Promise((resolve, reject) => {
-      stream.close(error => {
-        if (error) {
-          reject(error)
-        }
-
-        resolve()
-      })
-    })
-  }
-
   epilogue() {
     // We first let mocha.reporters.Spec do it's usual reporting using the default console defined on Base
     // Which means the report will be written to the terminal
@@ -57,11 +46,9 @@ class SpecFileReporter extends mocha.reporters.Spec {
 
     // We only output the epilogue to a file when we're in BUILDKITE and there are failures
     if (this.buildkite === true && this.failures.length > 0) {
-      const file = fs.createWriteStream(`${repoRoot}/annotations/${this.title}`)
       const customConsole = new Console({
-        stdout: file,
+        stdout: fs.createWriteStream(`./annotations/mocha-test-output-${this.title || 'placeholder'}`),
       })
-
       // We now want the Spec reporter (aka epilogue) to be written to a file, but Spec uses the console defined on Base!
       // So we swap out the consoleLog defined on Base with our customLog one
       // https://sourcegraph.com/github.com/mochajs/mocha/-/blob/lib/reporters/base.js?L43:5
@@ -71,23 +58,8 @@ class SpecFileReporter extends mocha.reporters.Spec {
       // https://mochajs.org/api/reporters_base.js.html#line367
       super.epilogue()
       // The report has been written to a file, so now we swap the consoleLog back to the originalConsole logger
+      // eslint-disable-next-line @typescript-eslint/unbound-method
       mocha.reporters.Base.consoleLog = originalConsoleLog
-      // We want to make sure before this reporter exits that the data written to file have been flushed
-      // In some scenarios, the node process exits too quickly and the data hasn't been flushed to the file yet
-      this.safeClose(file)
-        .then(() => {
-          const path = file.path.toString()
-          console.log(`${path} successfully closed`)
-        })
-        .catch(error => console.error(error))
-        .finally(() => {
-          console.warn('force exiting the process after writing report to file')
-          // This performs the same function as passing --exit to the mocha test runner
-          // When the regression tests run, some resources are not properly cleaned up. Leading to
-          // the test runner just hanging since it is waiting on an open resource to exit.
-          // TODO(burmudar): hunt this resource down
-          process.exit(1)
-        })
     }
   }
 }

client/web/package.json

@@ -6,8 +6,8 @@
   "license": "Apache-2.0",
   "scripts": {
     "test": "yarn run -T jest --testPathIgnorePatterns end-to-end --testPathIgnorePatterns regression integration",
-    "task:mocha": "yarn run -T cross-env TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha --config ../../.mocharc.js",
-    "test:regression": "yarn run task:mocha './src/regression/**/*.test.ts'",
+    "task:mocha": "yarn run -T cross-env TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha",
+    "test:regression": "yarn run task:mocha './src/regression/**/*.test.ts' --exit",
     "test:regression:codeintel": "yarn task:mocha ./src/regression/codeintel.test.ts",
     "test:regression:config-settings": "yarn task:mocha ./src/regression/config-settings.test.ts",
     "test:regression:core": "yarn task:mocha ./src/regression/core.test.ts",

dev/ci/integration/qa/test.sh

@@ -3,13 +3,7 @@
 export SOURCEGRAPH_BASE_URL="${1:-"http://localhost:7080"}"
 
 # shellcheck disable=SC1091
-if [[ $(id -u) -eq 1 ]]; then
-    source /root/.profile
-else
-    # shellcheck disable=SC1090
-    source "${HOME}/.profile"
-fi
-
+source /root/.profile
 cd "$(dirname "${BASH_SOURCE[0]}")/../../../.."
 
 set -e
@@ -22,16 +16,11 @@ popd
 # Load variables set up by init-server, disabling `-x` to avoid printing variables
 set +x
 # shellcheck disable=SC1091
-if [[ $(id -u) -eq 1 ]]; then
-    source /root/.sg_envrc
-else
-    # shellcheck disable=SC1090
-    source "${HOME}/.sg_envrc"
-fi
+source /root/.sg_envrc
 
 echo "--- TEST: Checking Sourcegraph instance is accessible"
-curl -f "${SOURCEGRAPH_BASE_URL}"
-curl -f "${SOURCEGRAPH_BASE_URL}/healthz"
+curl -f http://localhost:7080
+curl -f http://localhost:7080/healthz
 echo "--- TEST: Running tests"
 # Run all tests, and error if one fails
 test_status=0

enterprise/dev/ci/internal/ci/operations.go

@@ -327,12 +327,7 @@ func clientIntegrationTests(pipeline *bk.Pipeline) {
 			// If PERCY_PARALLEL_TOTAL is set, the API will wait for that many finalized builds to finalize the Percy build.
 			// https://docs.percy.io/docs/parallel-test-suites#how-it-works
 			bk.Env("PERCY_PARALLEL_TOTAL", strconv.Itoa(parallelTestCount)),
-			bk.AnnotatedCmd(fmt.Sprintf(`dev/ci/yarn-web-integration.sh "%s"`, chunkTestFiles), bk.AnnotatedCmdOpts{
-				Annotations: &bk.AnnotationOpts{
-					IncludeNames:    true,
-					MultiJobContext: "puppeteer",
-				},
-			}),
+			bk.Cmd(fmt.Sprintf(`dev/ci/yarn-web-integration.sh "%s"`, chunkTestFiles)),
 			bk.ArtifactPaths("./puppeteer/*.png"))
 	}
 }