fix(tests): Add const-to-var ts-jest transformer (#5022)

This does, during tests, what the `const`-to-`var` plugin in #4993 does for our built code. Though for tests bundle size isn't a concern, the first point raised there (of our shadowing of `global` being an issue now that we're ES6ified and living in a land of `const`s) stands, though in this case it seems to only affect Node 8.

Since `ts-jest` doesn't use our built code, but instead compiles our TS on the fly, the existing plugin is of no help. Hence this transformer, which `ts-jest `will apply after it has transpiled our code to JS. It works by providing a visitor function which `ts-jest` passes on to TS, which then lets it loose on the AST. In our case (as you'd expect) our visitor stops on every `const` variable declaration node and replaces it with an equivalent `var` declaration node, and ignores all other nodes. Since it's only needed for node 8, we leave transpilation of the transformer itself and injection into our jest config until runtime on the Node 8 CI machine. 

As part of this work, the script which runs tests in CI (whose main job is to ensure compatibility with older versions of Node, and to which the aforementioned transpilation and injection were added) has been reorganized into functions, for (hopefully) better readability.

Author: lobsterkatie

.gitignore

@@ -12,6 +12,8 @@ scratch/
 *.pyc
 *.tsbuildinfo
 scenarios/*/dist/
+# transpiled transformers
+jest/transformers/*.js
 
 # logs
 yarn-error.log

jest/transformers/constReplacer.ts

@@ -0,0 +1,84 @@
+/**
+ * This is a transformer which `ts-jest` applies during the compilation process, which switches all of the `const`s out
+ * for `var`s. Unlike in our package builds, where we do the same substiution for bundle size reasons, here we do it
+ * because otherwise `const global = getGlobalObject()` throws an error about redifining `global`. (This didn't used to
+ * be a problem because our down-compilation did the `const`-`var` substitution for us, but now that we're ES6-only, we
+ * have to do it ourselves.)
+ *
+ * Note: If you ever have to change this, and are testing it locally in the process, be sure to call
+ *     `yarn jest --clearCache`
+ * before each test run, as transformation results are cached between runs.
+ */
+
+import {
+  createVariableDeclarationList,
+  getCombinedNodeFlags,
+  isVariableDeclarationList,
+  Node,
+  NodeFlags,
+  SourceFile,
+  TransformationContext,
+  Transformer,
+  TransformerFactory,
+  visitEachChild,
+  visitNode,
+  VisitResult,
+} from 'typescript';
+
+// These can be anything - they're just used to construct a cache key for the transformer returned by the factory below.
+// This really only matters when you're testing the transformer itself, as changing these values gives you a quick way
+// to invalidate the cache and ensure that changes you've made to the code here are immediately picked up on and used.
+export const name = 'const-to-var';
+export const version = '1.0';
+
+/**
+ * Check whether the given AST node represents a `const` token.
+ *
+ * This function comes from the TS compiler, and is copied here to get around the fact that it's not exported by the
+ * `typescript` package.
+ *
+ * @param node The node to check
+ * @returns A boolean indicating if the node is a `const` token.
+ */
+function isVarConst(node: Node): boolean {
+  // eslint-disable-next-line no-bitwise
+  return !!(getCombinedNodeFlags(node) & NodeFlags.Const);
+}
+
+/**
+ * Return a set of nested factory functions, which ultimately creates an AST-node visitor function, which can modify
+ * each visited node as it sees fit, and uses it to walk the AST, returning the results.
+ *
+ * In our case, we're modifying all `const` variable declarations to use `var` instead.
+ */
+export function factory(): TransformerFactory<SourceFile> {
+  // Create the transformer
+  function transformerFactory(context: TransformationContext): Transformer<SourceFile> {
+    // Create a visitor function and use it to walk the AST
+    function transformer(sourceFile: SourceFile): SourceFile {
+      // This visitor function can either return a node, in which case the subtree rooted at the returned node is
+      // substituted for the subtree rooted at the visited node, or can use the recursive `visitEachChild` function
+      // provided by TS to continue traversing the tree.
+      function visitor(node: Node): VisitResult<Node> {
+        // If we've found a `const` declaration, return a `var` declaration in its place
+        if (isVariableDeclarationList(node) && isVarConst(node)) {
+          // A declaration list with a `None` flag defaults to using `var`
+          return createVariableDeclarationList(node.declarations, NodeFlags.None);
+        }
+
+        // This wasn't a node we're interested in, so keep walking down the tree.
+        return visitEachChild(node, visitor, context);
+      }
+
+      // Having defined our visitor, pass it to the TS-provided `visitNode` function, which will use it to walk the AST,
+      // and return the results of that walk.
+      return visitNode(sourceFile, visitor);
+    }
+
+    // Back in the transformer factory, return the transformer we just created
+    return transformer;
+  }
+
+  // Finally, we're back in `factory`, and can return the whole nested system
+  return transformerFactory;
+}

scripts/test.ts

@@ -1,66 +1,127 @@
-import { spawnSync } from 'child_process';
-import { join } from 'path';
+import * as childProcess from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
 
-function run(cmd: string, cwd: string = '') {
-  const result = spawnSync(cmd, { shell: true, stdio: 'inherit', cwd: join(__dirname, '..', cwd || '') });
+const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0];
 
-  if (result.status !== 0) {
-    process.exit(result.status || undefined);
-  }
-}
+// We run ember tests in their own job.
+const DEFAULT_SKIP_TESTS_PACKAGES = ['@sentry/ember'];
+// These packages don't support Node 8 for syntax or dependency reasons.
+const NODE_8_SKIP_TESTS_PACKAGES = [
+  ...DEFAULT_SKIP_TESTS_PACKAGES,
+  '@sentry-internal/eslint-plugin-sdk',
+  '@sentry/react',
+  '@sentry/wasm',
+  '@sentry/gatsby',
+  '@sentry/serverless',
+  '@sentry/nextjs',
+  '@sentry/angular',
+];
 
-const nodeMajorVersion = parseInt(process.version.split('.')[0].replace('v', ''), 10);
+// We have to downgrade some of our dependencies in order to run tests in Node 8 and 10.
+const NODE_8_LEGACY_DEPENDENCIES = [
+  '[email protected]',
+  '[email protected]',
+  '[email protected]',
+  '[email protected]',
+  '[email protected]',
+];
+const NODE_10_LEGACY_DEPENDENCIES = ['[email protected]'];
 
-// Ember tests require dependency changes for each set of tests, making them quite slow. To compensate for this, in CI
-// we run them in a separate, parallel job.
-let ignorePackages = ['@sentry/ember'];
+/**
+ * Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current
+ * process. Returns contents of `stdout`.
+ */
+function run(cmd: string, options?: childProcess.ExecSyncOptions) {
+  return childProcess.execSync(cmd, { stdio: 'inherit', ...options });
+}
 
-// install legacy versions of third-party packages whose current versions don't support node 8 or 10, and skip testing
-// our own packages which don't support node 8 for various syntax or dependency reasons
-if (nodeMajorVersion <= 10) {
-  let legacyDependencies;
+/**
+ * Install the given legacy dependencies, for compatibility with tests run in older versions of Node.
+ */
+function installLegacyDeps(legacyDeps: string[] = []): void {
+  // Ignoring engines and scripts lets us get away with having incompatible things installed for SDK packages we're not
+  // testing in the current node version, and ignoring the root check lets us install things at the repo root.
+  run(`yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check ${legacyDeps.join(' ')}`);
+}
 
-  if (nodeMajorVersion === 8) {
-    legacyDependencies = [
-      '[email protected]',
-      '[email protected]',
-      '[email protected]',
-      '[email protected]',
-      '[email protected]',
-    ];
+/**
+ * Add a tranformer to our jest config, to do the same `const`-to-`var` replacement as our rollup plugin does.
+ *
+ * This is needed because Node 8 doesn't like the way we shadow `global` (`const global = getGlobalObject()`). Changing
+ * it to a `var` solves this by making it redeclarable.
+ *
+ */
+function addTransformer(): void {
+  // Though newer `ts-jest` versions support transformers written in TS, the legacy version does not.
+  run('yarn tsc --skipLibCheck jest/transformers/constReplacer.ts');
 
-    ignorePackages = [
-      ...ignorePackages,
-      '@sentry-internal/eslint-plugin-sdk',
-      '@sentry/react',
-      '@sentry/wasm',
-      '@sentry/gatsby',
-      '@sentry/serverless',
-      '@sentry/nextjs',
-      '@sentry/angular',
-    ];
+  // Loading the existing Jest config will error out unless the config file has an accompanying types file, so we have
+  // to create that before we can load it.
+  run('yarn tsc --allowJs --skipLibCheck --declaration --emitDeclarationOnly jest/jest.config.js');
+  // eslint-disable-next-line @typescript-eslint/no-var-requires
+  const jestConfig = require('../jest/jest.config.js');
 
-    // This is a hack, to deal the fact that the browser-based tests fail under Node 8, because of a conflict buried
-    // somewhere in the interaction between our current overall set of dependencies and the older versions of a small
-    // subset we're about to install below. Since they're browser-based, these tests are never going to be running in a
-    // node 8 environment in any case, so it's fine to skip them here. (In the long run, we should only run such tests
-    // against a single version of node, but in the short run, this at least allows us to not be blocked by the
-    // failures.)
-    run('rm -rf packages/tracing/test/browser');
-  }
-  // Node 10
-  else {
-    legacyDependencies = ['[email protected]'];
-  }
+  // Inject the transformer
+  jestConfig.globals['ts-jest'].astTransformers = ['<rootDir>/../../jest/transformers/constReplacer.js'];
 
-  const legacyDepStr = legacyDependencies.join(' ');
+  // When we required the jest config file above, all expressions it contained were evaluated. Specifically, the
+  //     `rootDir: process.cwd()`
+  // entry was replaced with
+  //     `rootDir: "<hard-coded string result of running `process.cwd()` in the current process>"`,
+  // Though it's a little brute-force-y, the easiest way to fix this is to just stringify the code and perform the
+  // substitution in reverse.
+  const stringifiedConfig = JSON.stringify(jestConfig, null, 2).replace(
+    `"rootDir": "${process.cwd()}"`,
+    'rootDir: process.cwd()',
+  );
 
-  // ignoring engines and scripts lets us get away with having incompatible things installed for packages we're not testing
-  run(`yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check ${legacyDepStr}`);
+  // Now we just have to convert it back to a module and write it to disk
+  const code = `module.exports = ${stringifiedConfig}`;
+  fs.writeFileSync(path.resolve('jest/jest.config.js'), code);
 }
 
-const ignoreFlags = ignorePackages.map(dep => `--ignore="${dep}"`).join(' ');
+/**
+ * Skip tests which don't apply to Node and therefore don't need to run in older Node versions.
+ *
+ * TODO We're foreced to skip these tests for compatibility reasons (right now this function only gets called in Node
+ * 8), but we could be skipping a lot more tests in Node 8-14 - anything where compatibility with different Node
+ * versions is irrelevant - and only running them in Node 16.
+ */
+function skipNonNodeTests(): void {
+  run('rm -rf packages/tracing/test/browser');
+}
 
-run(`yarn test ${ignoreFlags}`);
+/**
+ * Run tests, ignoring the given packages
+ */
+function runWithIgnores(skipPackages: string[] = []): void {
+  const ignoreFlags = skipPackages.map(dep => `--ignore="${dep}"`).join(' ');
+  run(`yarn test ${ignoreFlags}`);
+}
+
+/**
+ * Run the tests, accounting for compatibility problems in older versions of Node.
+ */
+function runTests(): void {
+  if (CURRENT_NODE_VERSION === '8') {
+    installLegacyDeps(NODE_8_LEGACY_DEPENDENCIES);
+    // Inject a `const`-to-`var` transformer, in order to stop Node 8 from complaining when we shadow `global`
+    addTransformer();
+    // TODO Right now, this just skips incompatible tests, but it could be skipping more (hence the aspirational name),
+    // and not just in Node 8. See `skipNonNodeTests`'s docstring.
+    skipNonNodeTests();
+    runWithIgnores(NODE_8_SKIP_TESTS_PACKAGES);
+  }
+  //
+  else if (CURRENT_NODE_VERSION === '10') {
+    installLegacyDeps(NODE_10_LEGACY_DEPENDENCIES);
+    runWithIgnores(DEFAULT_SKIP_TESTS_PACKAGES);
+  }
+  //
+  else {
+    runWithIgnores(DEFAULT_SKIP_TESTS_PACKAGES);
+  }
+}
 
-process.exit(0);
+runTests();