fixup! feat(builtin): enable coverage on nodejs_test

bazel-contrib · Apr 25, 2020 · 3c4efec · 3c4efec
1 parent ddf1efb
commit 3c4efec
Show file tree

Hide file tree

Showing 14 changed files with 1,473 additions and 1,922 deletions.
diff --git a/common.bazelrc b/common.bazelrc
@@ -76,7 +76,8 @@ common --experimental_allow_incremental_repository_updates
 build --incompatible_strict_action_env
 run --incompatible_strict_action_env
 
-# when running `bazel coverage` ensure that the test targets are instrumented 
+# When running `bazel coverage` --instrument_test_targets needs to be set in order to 
+# collect coverage information from test targets
 coverage --instrument_test_targets
 
 # Load any settings specific to the current user.

diff --git a/internal/coverage/BUILD.bazel b/internal/coverage/BUILD.bazel
@@ -1,12 +1,16 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
 
+exports_files([
+    "lcov_merger-js.js",
+])
+
 # BEGIN-INTERNAL
-load("@npm_bazel_typescript//:index.from_src.bzl", "checked_in_ts_library")
+load("@npm_bazel_typescript//:checked_in_ts_project.bzl", "checked_in_ts_project")
 
-checked_in_ts_library(
+checked_in_ts_project(
     name = "lcov_merger_js_lib",
-    srcs = ["lcov_merger.ts"],
-    checked_in_js = "lcov_merger.js",
+    src = "lcov_merger.ts",
+    checked_in_js = "lcov_merger-js.js",
     visibility = ["//visibility:public"],
     deps = ["@npm//@types/node"],
 )

diff --git a/internal/coverage/lcov_merger-js.js b/internal/coverage/lcov_merger-js.js
@@ -0,0 +1,70 @@
+/* THIS FILE GENERATED FROM .ts; see BUILD.bazel */ /* clang-format off */"use strict";
+var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
+    function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
+    return new (P || (P = Promise))(function (resolve, reject) {
+        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
+        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
+        function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); }
+        step((generator = generator.apply(thisArg, _arguments || [])).next());
+    });
+};
+Object.defineProperty(exports, "__esModule", { value: true });
+const crypto = require("crypto");
+const fs = require("fs");
+const path = require("path");
+function _getArg(argv, key) {
+    return argv.find(a => a.startsWith(key)).split('=')[1];
+}
+function main() {
+    return __awaiter(this, void 0, void 0, function* () {
+        const argv = process.argv;
+        const coverageDir = _getArg(argv, '--coverage_dir=');
+        const outputFile = _getArg(argv, '--output_file=');
+        const sourceFileManifest = _getArg(argv, '--source_file_manifest=');
+        const tmpdir = process.env.TEST_TMPDIR;
+        if (!sourceFileManifest || !tmpdir || !outputFile) {
+            throw new Error();
+        }
+        const instrumentedSourceFiles = fs.readFileSync(sourceFileManifest).toString('utf8').split('\n');
+        const c8OutputDir = path.join(tmpdir, crypto.randomBytes(4).toString('hex'));
+        fs.mkdirSync(c8OutputDir);
+        const includes = instrumentedSourceFiles
+            .filter(f => ['.js', '.jsx', '.cjs', '.ts', '.tsx', '.mjs'].includes(path.extname(f)))
+            .map(f => {
+            const p = path.parse(f);
+            let targetExt;
+            switch (p.ext) {
+                case '.mjs':
+                    targetExt = '.mjs';
+                default:
+                    targetExt = '.js';
+            }
+            return path.format(Object.assign(Object.assign({}, p), { base: undefined, ext: targetExt }));
+        });
+        let c8;
+        try {
+            c8 = require('c8');
+        }
+        catch (e) {
+            if (e.code == 'MODULE_NOT_FOUND') {
+                console.error('ERROR: c8 npm package is required for bazel coverage');
+                process.exit(1);
+            }
+            throw e;
+        }
+        yield new c8
+            .Report({
+            include: includes,
+            exclude: includes.length === 0 ? ['**'] : [],
+            reportsDirectory: c8OutputDir,
+            tempDirectory: coverageDir,
+            resolve: '',
+            reporter: ['lcovonly']
+        })
+            .run();
+        fs.copyFileSync(path.join(c8OutputDir, 'lcov.info'), outputFile);
+    });
+}
+if (require.main === module) {
+    main();
+}
diff --git a/internal/coverage/lcov_merger.js b/internal/coverage/lcov_merger.js
diff --git a/internal/coverage/lcov_merger.ts b/internal/coverage/lcov_merger.ts
@@ -19,32 +19,30 @@ import * as crypto from 'crypto';
 import * as fs from 'fs';
 import * as path from 'path';
 
-function getArg(argv: string[], key: string): string {
-  return argv.find(a => a === key)!.split('=')[1];
+function _getArg(argv: string[], key: string): string {
+  return argv.find(a => a.startsWith(key))!.split('=')[1];
 }
 
 /**
  * This is designed to collect the coverage of one target, since in nodejs
  * and using NODE_V8_COVERAGE it may produce more than one coverage file, however bazel expects
  * there to be only one lcov file. So this collects up the v8 coverage json's merges them and
  * converts them to lcov for bazel to pick up later.
- * any tool reporting coverage not just jasmine
  */
 async function main() {
-  // see here for what args are passed in
+  // Using the standard args for a bazel lcov merger binary:
   // https://github.com/bazelbuild/bazel/blob/master/tools/test/collect_coverage.sh#L175-L181
   const argv = process.argv;
-  const coverageDir = getArg(argv, 'coverage_dir');
-  const outputFile = getArg(argv, 'output_file');
-  const sourceFileManifest = getArg(argv, 'source_file_manifest');
+  const coverageDir = _getArg(argv, '--coverage_dir=');
+  const outputFile = _getArg(argv, '--output_file=');
+  const sourceFileManifest = _getArg(argv, '--source_file_manifest=');
   const tmpdir = process.env.TEST_TMPDIR;
 
   if (!sourceFileManifest || !tmpdir || !outputFile) {
     throw new Error();
   }
 
   const instrumentedSourceFiles = fs.readFileSync(sourceFileManifest).toString('utf8').split('\n');
-
   // c8 will name the output report file lcov.info
   // so we give it a dir that it can write to
   // later on we'll move and rename it into output_file as bazel expects
@@ -55,12 +53,12 @@ async function main() {
       instrumentedSourceFiles
           // the manifest may include files such as .bash so we want to reduce that down to the set
           // we can run coverage on in JS
-          .filter(f => ['.js', '.jsx', 'cjs', '.ts', '.tsx', '.mjs'].includes(path.extname(f)))
+          .filter(f => ['.js', '.jsx', '.cjs', '.ts', '.tsx', '.mjs'].includes(path.extname(f)))
           .map(f => {
             // at runtime we only run .js or .mjs
             // meaning that the coverage written by v8 will only include urls to .js or .mjs
             // so the source files need to be mapped from their input to output extensions
-            // TODO: how do we know what source files produce .mjs or cjs?
+            // TODO: how do we know what source files produce .mjs or .cjs?
             const p = path.parse(f);
             let targetExt;
             switch (p.ext) {
@@ -74,7 +72,16 @@ async function main() {
           });
 
   // only require in c8 when we're actually going to do some coverage
-  const c8 = require('c8');
+  let c8;
+  try {
+    c8 = require('c8');
+  } catch (e) {
+    if (e.code == 'MODULE_NOT_FOUND') {
+      console.error('ERROR: c8 npm package is required for bazel coverage');
+      process.exit(1);
+    }
+    throw e;
+  }
   // see https://github.com/bcoe/c8/blob/master/lib/report.js
   // for more info on this function
   // TODO: enable the --all functionality

diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh
@@ -90,9 +90,6 @@ fi
 export RUNFILES
 # --- end RUNFILES initialization ---
 
-# TODO: debug - remove this
-set -x
-
 TEMPLATED_env_vars
 
 # Note: for debugging it is useful to see what files are actually present
@@ -161,6 +158,7 @@ MAIN=$(rlocation "TEMPLATED_loader_script")
 
 readonly repository_args=$(rlocation "TEMPLATED_repository_args")
 readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script")
+readonly lcov_merger_script=$(rlocation "TEMPLATED_lcov_merger_script")
 node_patches_script=$(rlocation "TEMPLATED_node_patches_script")
 require_patch_script=${BAZEL_NODE_PATCH_REQUIRE}
 
@@ -257,31 +255,67 @@ fi
 # a binary fails to run. Otherwise any failure would make such a test
 # fail before we could assert that we expected that failure.
 readonly EXPECTED_EXIT_CODE="TEMPLATED_expected_exit_code"
-if [ "${EXPECTED_EXIT_CODE}" -eq "0" ]; then
-  # Replace the current process (bash) with a node process.
-  # This means that stdin, stdout, signals, etc will be transparently
-  # handled by the node process.
-  # If we had merely forked a child process here, we'd be responsible
-  # for forwarding those OS interactions.
-  exec "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
-  # exec terminates execution of this shell script, nothing later will run.
+
+if [[ -n "${COVERAGE_DIR:-}" ]]; then
+  if [[ -n "${VERBOSE_LOGS:-}" ]]; then
+    echo "Turning on node coverage with NODE_V8_COVERAGE=${COVERAGE_DIR}"
+  fi
+  # Setting NODE_V8_COVERAGE=${COVERAGE_DIR} causes NodeJS to write coverage
+  # information our to the COVERAGE_DIR once  the process exits
+  export NODE_V8_COVERAGE=${COVERAGE_DIR}
 fi
 
+# Bash does not forward termination signals to any child process when
+# running in docker so need to manually trap and forward the signals
+_term() {
+  kill -TERM "$child" 2>/dev/null
+}
+
+_int() {
+  kill -INT "$child" 2>/dev/null
+}
+
+# Execute the main program
 set +e
-"${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
+"${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 &
+child=$!
+trap _term SIGTERM
+trap _int SIGINT
+wait "$child"
 RESULT="$?"
 set -e
 
-if [ ${RESULT} != ${EXPECTED_EXIT_CODE} ]; then
-  echo "Expected exit code to be ${EXPECTED_EXIT_CODE}, but got ${RESULT}" >&2
-  if [ "${RESULT}" -eq "0" ]; then
-    # This exit code is handled specially by Bazel:
-    # https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44
-    readonly BAZEL_EXIT_TESTS_FAILED=3;
-    exit ${BAZEL_EXIT_TESTS_FAILED}
+if [ "${EXPECTED_EXIT_CODE}" != "0" ]; then
+  if [ ${RESULT} != ${EXPECTED_EXIT_CODE} ]; then
+    echo "Expected exit code to be ${EXPECTED_EXIT_CODE}, but got ${RESULT}" >&2
+    if [ "${RESULT}" -eq "0" ]; then
+      # This exit code is handled specially by Bazel:
+      # https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44
+      readonly BAZEL_EXIT_TESTS_FAILED=3;
+      exit ${BAZEL_EXIT_TESTS_FAILED}
+    fi
+  else 
+    exit 0
+  fi
+fi
+
+# Post process the coverage information after the process has exited
+if [[ -n "${COVERAGE_DIR:-}" ]]; then
+  if [[ -n "${VERBOSE_LOGS:-}" ]]; then
+    echo "Running coverage lcov merger script with arguments"
+    echo "  --coverage_dir="${COVERAGE_DIR}""
+    echo "  --output_file="${COVERAGE_OUTPUT_FILE}""
+    echo "  --source_file_manifest="${COVERAGE_MANIFEST}""
+  fi
+
+  set +e
+  "${node}" "${lcov_merger_script}" --coverage_dir="${COVERAGE_DIR}" --output_file="${COVERAGE_OUTPUT_FILE}" --source_file_manifest="${COVERAGE_MANIFEST}"
+  RESULT="$?"
+  set -e
+
+  if [ ${RESULT} -ne 0 ]; then
+    exit ${RESULT}
   fi
-else
-  exit 0
 fi
 
 exit ${RESULT}