From 63de6a4ab7de634de715e3334864529f08f6e151 Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Thu, 8 Sep 2022 12:48:01 -0700 Subject: [PATCH] =?UTF-8?q?Revert=20"regression-tests:=20update=20custom?= =?UTF-8?q?=20mocha=20reporter=20to=20exit=20rather=E2=80=A6=20(#41542)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert "regression-tests: update custom mocha reporter to exit rather than mocha itself (#41266)" This reverts commit 3b31a19dcb76733f6e63d95fa8488ee1cac6680d. This change's build's QA test timed out (https://buildkite.com/sourcegraph/sourcegraph/builds/172014#01831e2c-78c0-4046-8270-b3a8f5d48a90), the previous build https://github.com/sourcegraph/sourcegraph/commit/aa2c9830f151ff0490409675662412b8ce4a1053 did not. The QA build hanging is causing all builds in main to fail after timing out. --- .mocharc.js | 10 ++--- client/shared/dev/customMochaSpecReporter.js | 42 ++++---------------- client/web/package.json | 4 +- dev/ci/integration/qa/test.sh | 19 ++------- enterprise/dev/ci/internal/ci/operations.go | 7 +--- 5 files changed, 17 insertions(+), 65 deletions(-) diff --git a/.mocharc.js b/.mocharc.js index 210a5eb9a0e..cb7ff7f3f19 100644 --- a/.mocharc.js +++ b/.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 diff --git a/client/shared/dev/customMochaSpecReporter.js b/client/shared/dev/customMochaSpecReporter.js index b68e20fdf8f..f3612a10891 100644 --- a/client/shared/dev/customMochaSpecReporter.js +++ b/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) - }) } } } diff --git a/client/web/package.json b/client/web/package.json index e6501a9bd3b..3f0a17f8f12 100644 --- a/client/web/package.json +++ b/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", diff --git a/dev/ci/integration/qa/test.sh b/dev/ci/integration/qa/test.sh index 0592accc656..f2954ef352a 100755 --- a/dev/ci/integration/qa/test.sh +++ b/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 diff --git a/enterprise/dev/ci/internal/ci/operations.go b/enterprise/dev/ci/internal/ci/operations.go index 91b7dd0d919..9c3472f77d0 100644 --- a/enterprise/dev/ci/internal/ci/operations.go +++ b/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")) } }