feat: add default DEBUG and DEBUG_LOGS configuration_env_vars to node…

…js_binary

all rules updated to use DEBUG and DEBUG_LOGS environment variables

.bazelci/presubmit.yml

@@ -106,6 +106,16 @@ tasks:
     - "--test_tag_filters=-e2e,-examples,-fix-bazelci-ubuntu"
     test_targets:
     - "//..."
+  ubuntu1804_debug:
+    name: ubuntu1804_debug
+    platform: ubuntu1804
+    test_flags:
+    - "--define=DEBUG_LOGS=true"
+    - "--define=DEBUG=true"
+    - "--test_tag_filters=-e2e,-examples,-fix-bazelci-ubuntu"
+    test_targets:
+    - "//..."
+    - "//packages/terser/test/debug:test_define_DEBUG"
   ubuntu1804_e2e:
     name: ubuntu1804_e2e
     platform: ubuntu1804

.circleci/config.yml

@@ -152,7 +152,22 @@ jobs:
       - *init_bazel
       - *hide_node_and_yarn_local_binaries
 
-      - run: bazel test ... --test_tag_filters=-e2e,-examples
+      - run: bazel test --test_tag_filters=-e2e,-examples ...
+
+      - store_artifacts:
+          path: dist/bin/release.tar.gz
+          destination: release.tar.gz
+
+  test_cebug:
+    <<: *job_defaults
+    resource_class: xlarge
+    steps:
+      - *attach_workspace
+      - *init_environment
+      - *init_bazel
+      - *hide_node_and_yarn_local_binaries
+
+      - run: bazel test --define=DEBUG=true --define=DEBUG_LOGS=true --test_tag_filters=-e2e,-examples ... //packages/terser/test/debug:test_define_DEBUG
 
       - store_artifacts:
           path: dist/bin/release.tar.gz
@@ -298,6 +313,9 @@ workflows:
       - test:
           requires:
           - setup
+      - test_debug:
+          requires:
+          - setup
       - test_e2e:
           requires:
           - setup

internal/bazel_integration_test/test_runner.js

@@ -15,8 +15,6 @@
  * limitations under the License.
  */
 
-const DEBUG = false;
-
 // Set TEST_MANIFEST to true and use `bazel run` to excersize the MANIFEST
 // file code path on Linux and OSX
 const TEST_MANIFEST = false;
@@ -26,17 +24,28 @@ const fs = require('fs');
 const path = require('path');
 const tmp = require('tmp');
 
+const DEBUG = !!process.env['DEBUG'];
+const DEBUG_LOGS = !!process.env['DEBUG_LOGS'];
+
+function info(...m) {
+  console.error('[test_runner.js]', ...m);
+}
+
+function debug(...m) {
+  if (DEBUG_LOGS) console.error('[test_runner.js]', ...m);
+}
+
 const config = require(process.argv[2]);
-if (DEBUG) console.log(`config: ${JSON.stringify(config, null, 2)}`);
+debug(`config: ${JSON.stringify(config, null, 2)}`);
 
 const testArgs = process.argv.slice(3);
-if (DEBUG) console.log(`testArgs: ${JSON.stringify(testArgs, null, 2)}`);
+debug(`testArgs: ${JSON.stringify(testArgs, null, 2)}`);
 
 /**
- * Helper function to log out the contents of a file.
+ * Helper function to debug log out the contents of a file.
  */
-function logFileContents(desc, contents) {
-  console.log(`\n\n${
+function debugFileContents(desc, contents) {
+  debug(`${
       desc}\n========================================================================================\n${
       contents}\n^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n`);
 }
@@ -82,7 +91,7 @@ function copyFolderSync(from, to) {
     if (fs.statSync(src).isFile()) {
       mkdirp(path.dirname(dest));
       fs.copyFileSync(src, dest);
-      if (DEBUG) console.log(`copying ${src} -> ${dest}`);
+      debug(`copying ${src} -> ${dest}`);
     } else {
       copyFolderSync(src, dest);
     }
@@ -130,7 +139,7 @@ function copyWorkspace(workspace) {
         const element = key.slice(start.length);
         const dest = path.posix.join(to, element);
         mkdirp(path.dirname(dest));
-        if (DEBUG) console.log(`copying (MANIFEST) ${RUNFILES_MANIFEST[key]} -> ${dest}`);
+        debug(`copying (MANIFEST) ${RUNFILES_MANIFEST[key]} -> ${dest}`);
         fs.copyFileSync(RUNFILES_MANIFEST[key], dest);
       }
     }
@@ -158,7 +167,7 @@ function copyNpmPackage(packagePath) {
   return to;
 }
 
-if (DEBUG) console.log(`\n\ncopying workspace under test ${config.workspaceUnderTest} to tmp`);
+debug(`copying workspace under test ${config.workspaceUnderTest} to tmp`);
 const workspaceRoot = copyWorkspace(config.workspaceUnderTest);
 
 // Handle .bazelrc import replacements
@@ -172,7 +181,7 @@ if (bazelrcImportsKeys.length && isFile(bazelrcFile)) {
     bazelrcContents = bazelrcContents.replace(importKey, importContents);
   }
   fs.writeFileSync(bazelrcFile, bazelrcContents);
-  if (DEBUG) logFileContents('.bazelrc file with replacements:', bazelrcContents);
+  debugFileContents('.bazelrc file with replacements:', bazelrcContents);
 }
 
 // Handle appending to .bazelrc
@@ -182,7 +191,7 @@ if (config.bazelrcAppend) {
   bazelrcContents += '\n\n# Appended by bazel_integration_test\n';
   bazelrcContents += config.bazelrcAppend;
   fs.writeFileSync(bazelrcFile, bazelrcContents);
-  if (DEBUG) logFileContents('.bazelrc file after appending:', bazelrcContents);
+  debugFileContents('.bazelrc file after appending:', bazelrcContents);
 }
 
 // Handle WORKSPACE replacements
@@ -206,7 +215,7 @@ if (config.bazelrcAppend) {
     }
   }
   fs.writeFileSync(workspaceFile, workspaceContents);
-  if (DEBUG) logFileContents('WORKSPACE file with replacements:', workspaceContents);
+  debugFileContents('WORKSPACE file with replacements:', workspaceContents);
 }
 
 // Handle package.json replacements
@@ -217,7 +226,7 @@ if (isFile(packageJsonFile)) {
   const npmPackageKeys = Object.keys(config.npmPackages);
   if (npmPackageKeys.length) {
     for (const packageJsonKey of npmPackageKeys) {
-      if (DEBUG) console.log(`\n\ncopying npm package ${packageJsonKey} to tmp`);
+      debug(`copying npm package ${packageJsonKey} to tmp`);
       const packagePath = copyNpmPackage(config.npmPackages[packageJsonKey]).replace(/\\/g, '/');
       const regex = new RegExp(`\"${packageJsonKey}\"\\s*\:\\s*\"[^"]+`)
       const replacement = `"${packageJsonKey}": "file:${packagePath}`;
@@ -255,21 +264,21 @@ if (isFile(packageJsonFile)) {
     }
   }
 
-  if (DEBUG) logFileContents('package.json file with replacements:', packageJsonContents);
+  debugFileContents('package.json file with replacements:', packageJsonContents);
 }
 
 const isWindows = process.platform === 'win32';
 const bazelBinary =
     require.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`);
 
-console.log(`\n\nRunning 'bazel version'`);
+info(`running 'bazel version'`);
 let spawnedProcess = spawnSync(bazelBinary, ['version'], {cwd: workspaceRoot, stdio: 'inherit'});
 if (spawnedProcess.status) {
   process.exit(spawnedProcess.status);
 }
 
-if (DEBUG) {
-  console.log(`\n\nRunning 'bazel info'`);
+if (DEBUG_LOGS) {
+  debug(`running 'bazel info'`);
   spawnedProcess = spawnSync(bazelBinary, ['info'], {cwd: workspaceRoot, stdio: 'inherit'});
   if (spawnedProcess.status) {
     process.exit(spawnedProcess.status);
@@ -286,7 +295,7 @@ for (const bazelCommand of config.bazelCommands) {
   } else {
     bazelArgs.push(...testArgs);
   }
-  console.log(`\n\nRunning 'bazel ${bazelArgs.join(' ')}'`);
+  info(`running 'bazel ${bazelArgs.join(' ')}'`);
   spawnedProcess = spawnSync(bazelBinary, bazelArgs, {cwd: workspaceRoot, stdio: 'inherit'});
   if (spawnedProcess.status) {
     process.exit(spawnedProcess.status);

internal/node/node.bzl

@@ -156,7 +156,7 @@ def _nodejs_binary_impl(ctx):
             ctx.outputs.loader.short_path,
         ])
     env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
-    for k in ctx.attr.configuration_env_vars:
+    for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars:
         if k in ctx.var.keys():
             env_vars += "export %s=\"%s\"\n" % (k, ctx.var[k])
 
@@ -269,6 +269,19 @@ _NODEJS_EXECUTABLE_ATTRS = {
         allow_files = True,
         aspects = [sources_aspect, module_mappings_runtime_aspect, collect_node_modules_aspect],
     ),
+    "default_env_vars": attr.string_list(
+        doc = """Default environment variables that are added to `configuration_env_vars`.
+
+        This is separate from the default of `configuration_env_vars` so that a user can set `configuration_env_vars`
+        without losing the defaults that should be set in most cases.
+        
+        The set of default  environment variables is:
+
+        `DEBUG`: rules use this environment variable to turn on debug information in their output artifacts
+        `DEBUG_LOGS`: rules use this environment variable to turn on debug output in their logs
+        """,
+        default = ["DEBUG", "DEBUG_LOGS"],
+    ),
     "entry_point": attr.label(
         doc = """The script which should be executed first, usually containing a main function.