Let CI distinguish test compilation and test run failures (#15600)

Previously, `npm run test --output` exited with a non-zero code on both
compilation and test errors. This made it impossible to determine which
of the two happened - and thus hard to decide whether to generate test
reports. Now, the command exits with 0 on test failures. (Note that the
behavior is unchanged when --output is not given. That is, in this case
you still get non-zero exit codes for test failures.)

build/commands/lib/applyPatches.js

@@ -1,7 +1,7 @@
 const config = require('../lib/config')
 const util = require('../lib/util')
 
-const applyPatches = (buildConfig = config.defaultBuildConfig, options) => {
+const applyPatches = (buildConfig = config.defaultBuildConfig, options = {}) => {
   async function RunCommand () {
     config.buildConfig = buildConfig
     config.update(options)

build/commands/lib/build.js

@@ -18,7 +18,7 @@ const checkVersionsMatch = () => {
   }
 }
 
-const build = (buildConfig = config.defaultBuildConfig, options) => {
+const build = (buildConfig = config.defaultBuildConfig, options = {}) => {
   config.buildConfig = buildConfig
   config.update(options)
   checkVersionsMatch()

build/commands/lib/createDist.js

@@ -3,7 +3,7 @@ const util = require('../lib/util')
 const path = require('path')
 const fs = require('fs-extra')
 
-const createDist = (buildConfig = config.defaultBuildConfig, options) => {
+const createDist = (buildConfig = config.defaultBuildConfig, options = {}) => {
   config.buildConfig = buildConfig
   config.update(options)
   util.updateBranding()

build/commands/lib/gnCheck.js

@@ -1,7 +1,7 @@
 const config = require('../lib/config')
 const util = require('../lib/util')
 
-const gnCheck = (buildConfig = config.defaultBuildConfig, options) => {
+const gnCheck = (buildConfig = config.defaultBuildConfig, options = {}) => {
   config.buildConfig = buildConfig
   config.update(options)
   util.run('gn', ['check', config.outputDir, '//brave/*'], config.defaultOptions)

build/commands/lib/test.js

@@ -53,7 +53,30 @@ const getApplicableFilters = (suite) => {
   return filterFilePaths
 }
 
-const test = (passthroughArgs, suite, buildConfig = config.defaultBuildConfig, options) => {
+const test = (passthroughArgs, suite, buildConfig = config.defaultBuildConfig, options = {}) => {
+  buildTests(suite, buildConfig, options)
+  runTests(passthroughArgs, suite, buildConfig, options)
+}
+
+const buildTests = (suite, buildConfig = config.defaultBuildConfig, options = {}) => {
+  config.buildConfig = buildConfig
+  config.update(options)
+
+  let testSuites = [
+    'brave_unit_tests',
+    'brave_browser_tests',
+    'brave_network_audit_tests',
+  ]
+  if (testSuites.includes(suite)) {
+    config.buildTarget = 'brave/test:' + suite
+  } else {
+    config.buildTarget = suite
+  }
+  util.touchOverriddenFilesAndUpdateBranding()
+  util.buildTarget()
+}
+
+const runTests = (passthroughArgs, suite, buildConfig, options) => {
   config.buildConfig = buildConfig
   config.update(options)
 
@@ -96,20 +119,6 @@ const test = (passthroughArgs, suite, buildConfig = config.defaultBuildConfig, o
 
   braveArgs = braveArgs.concat(passthroughArgs)
 
-  // Build the tests
-  let testSuites = [
-    'brave_unit_tests',
-    'brave_browser_tests',
-    'brave_network_audit_tests',
-  ]
-  if (testSuites.includes(suite)) {
-    config.buildTarget = 'brave/test:' + suite
-  } else {
-    config.buildTarget = suite
-  }
-  util.touchOverriddenFilesAndUpdateBranding()
-  util.buildTarget()
-
   // Filter out upstream tests that are known to fail for Brave
   let upstreamTestSuites = [
     'unit_tests',
@@ -129,7 +138,7 @@ const test = (passthroughArgs, suite, buildConfig = config.defaultBuildConfig, o
     ], config.defaultOptions)
   } else {
     // Run the tests
-    getTestsToRun(config, suite).forEach((testSuite) => {
+    getTestsToRun(config, suite).every((testSuite) => {
       if (options.output) {
         braveArgs.splice(braveArgs.indexOf('--gtest_output=xml:' + options.output), 1)
         braveArgs.push(`--gtest_output=xml:${testSuite}.xml`)
@@ -143,7 +152,21 @@ const test = (passthroughArgs, suite, buildConfig = config.defaultBuildConfig, o
         // Specify emulator to run tests on
         braveArgs.push(`--avd-config tools/android/avd/proto/generic_android28.textpb`)
       }
-      util.run(path.join(config.outputDir, getTestBinary(testSuite)), braveArgs, config.defaultOptions)
+      let runOptions = config.defaultOptions
+      if (options.output)
+        // When test results are saved to a file, callers (such as CI) generate
+        // and analyze test reports as a next step. These callers are typically
+        // not interested in the exit code of running the tests, because they
+        // get the information about test success or failure from the output
+        // file. On the other hand, callers are interested in errors that
+        // produce an exit code, such as test compilation failures. By ignoring
+        // the test exit code here, we give callers a chance to distinguish test
+        // failures (by looking at the output file) from compilation errors.
+        runOptions.continueOnFail = true
+      let prog = util.run(path.join(config.outputDir, getTestBinary(testSuite)), braveArgs, runOptions)
+      // Don't run other tests if one has failed already, especially because
+      // this would overwrite the --output file (if given).
+      return prog.status === 0
     })
   }
 }

build/commands/scripts/commands.js

@@ -73,7 +73,7 @@ program
   .option('--target_os <target_os_type>', 'target OS type', /^(host_os|ios|android)$/i)
   .option('--target_arch <target_arch>', 'target architecture', /^(host_cpu|x64|arm64|x86)$/i)
   .arguments('[build_config]')
-  .action((buildConfig = config.defaultBuildConfig, options) => {
+  .action((buildConfig = config.defaultBuildConfig, options = {}) => {
     config.buildConfig = buildConfig
     if (options.target_os == 'host_os')
       delete options.target_os