Merge pull request #921 from gemini-testing/TESTPLANE-13.babel_imports

fix: dont remove imports in nodejs env
gemini-testing · May 6, 2024 · d2e06e2 · d2e06e2
2 parents 728f6bb + f132eeb
commit d2e06e2
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 30 deletions.
diff --git a/src/bundle/test-transformer.ts b/src/bundle/test-transformer.ts
@@ -6,7 +6,7 @@ import { TRANSFORM_EXTENSIONS, JS_EXTENSION_RE } from "./constants";
 import type { NodePath, PluginObj, TransformOptions } from "@babel/core";
 import type { ImportDeclaration } from "@babel/types";
 
-export const setupTransformHook = (): VoidFunction => {
+export const setupTransformHook = (opts: { removeNonJsImports?: boolean } = {}): VoidFunction => {
     const transformOptions: TransformOptions = {
         browserslistConfigFile: false,
         babelrc: false,
@@ -22,23 +22,26 @@ export const setupTransformHook = (): VoidFunction => {
                 },
             ],
             require("@babel/plugin-transform-modules-commonjs"),
-            [
-                (): PluginObj => ({
-                    name: "ignore-imports",
-                    visitor: {
-                        ImportDeclaration(path: NodePath<ImportDeclaration>): void {
-                            const extname = nodePath.extname(path.node.source.value);
-
-                            if (extname && !extname.match(JS_EXTENSION_RE)) {
-                                path.remove();
-                            }
-                        },
-                    },
-                }),
-            ],
         ],
     };
 
+    const customIgnoreImportsPlugin = (): PluginObj => ({
+        name: "ignore-imports",
+        visitor: {
+            ImportDeclaration(path: NodePath<ImportDeclaration>): void {
+                const extname = nodePath.extname(path.node.source.value);
+
+                if (extname && !extname.match(JS_EXTENSION_RE)) {
+                    path.remove();
+                }
+            },
+        },
+    });
+
+    if (opts.removeNonJsImports) {
+        transformOptions.plugins!.push([customIgnoreImportsPlugin]);
+    }
+
     const revertTransformHook = addHook(
         (originalCode, filename) => {
             return babel.transform(originalCode, { filename, ...transformOptions })!.code as string;

diff --git a/src/test-reader/index.js b/src/test-reader/index.js
@@ -30,7 +30,11 @@ module.exports = class TestReader extends EventEmitter {
             .useBrowsers(browsers)
             .build(process.cwd(), { ignore }, fileExtensions);
 
-        const parser = new TestParser();
+        const testRunEnv = _.isArray(this.#config.system.testRunEnv)
+            ? this.#config.system.testRunEnv[0]
+            : this.#config.system.testRunEnv;
+
+        const parser = new TestParser({ testRunEnv });
         passthroughEvent(parser, this, [MasterEvents.BEFORE_FILE_READ, MasterEvents.AFTER_FILE_READ]);
 
         await parser.loadFiles(setCollection.getAllFiles(), this.#config);

diff --git a/src/test-reader/test-parser.js b/src/test-reader/test-parser.js
@@ -15,11 +15,17 @@ const clearRequire = require("clear-require");
 const path = require("path");
 
 class TestParser extends EventEmitter {
+    #opts;
     #buildInstructions;
 
-    constructor() {
+    /**
+     * @param {object} opts
+     * @param {"nodejs" | "browser" | undefined} opts.testRunEnv - environment to parse tests for
+     */
+    constructor(opts = {}) {
         super();
 
+        this.#opts = opts;
         this.#buildInstructions = new InstructionsList();
     }
 
@@ -51,7 +57,7 @@ class TestParser extends EventEmitter {
 
         this.#clearRequireCache(files);
 
-        const revertTransformHook = setupTransformHook();
+        const revertTransformHook = setupTransformHook({ removeNonJsImports: this.#opts.testRunEnv === "browser" });
 
         const rand = Math.random();
         const esmDecorator = f => f + `?rand=${rand}`;

diff --git a/src/test-reader/test-transformer.ts b/src/test-reader/test-transformer.ts
@@ -1,5 +1,6 @@
 /* eslint-disable @typescript-eslint/no-var-requires */
 
 // TODO: use import instead require. Currently require is used because ts build filed from bundle folder even if it excluded in tsconfig
-export const setupTransformHook: () => VoidFunction = require("../bundle").setupTransformHook;
+export const setupTransformHook: (opts?: { removeNonJsImports?: boolean }) => VoidFunction =
+    require("../bundle").setupTransformHook;
 export const TRANSFORM_EXTENSIONS: string[] = require("../bundle").TRANSFORM_EXTENSIONS;
diff --git a/src/worker/runner/simple-test-parser.js b/src/worker/runner/simple-test-parser.js
@@ -1,5 +1,6 @@
 "use strict";
 
+const _ = require("lodash");
 const { EventEmitter } = require("events");
 const { passthroughEvent } = require("../../events/utils");
 const { TestParser } = require("../../test-reader/test-parser");
@@ -17,7 +18,11 @@ module.exports = class SimpleTestParser extends EventEmitter {
     }
 
     async parse({ file, browserId }) {
-        const parser = new TestParser();
+        const testRunEnv = _.isArray(this._config.system.testRunEnv)
+            ? this._config.system.testRunEnv[0]
+            : this._config.system.testRunEnv;
+
+        const parser = new TestParser({ testRunEnv });
 
         passthroughEvent(parser, this, [WorkerEvents.BEFORE_FILE_READ, WorkerEvents.AFTER_FILE_READ]);
 

diff --git a/test/src/test-reader/test-parser.js b/test/src/test-reader/test-parser.js
@@ -374,6 +374,21 @@ describe("test-reader/test-parser", () => {
             });
 
             describe("transform hook", () => {
+                describe("removeNonJsImports", () => {
+                    [
+                        { removeNonJsImports: true, testRunEnv: "browser" },
+                        { removeNonJsImports: false, testRunEnv: "nodejs" },
+                    ].forEach(({ removeNonJsImports, testRunEnv }) => {
+                        it(`should be "${removeNonJsImports}" if testRunEnv is "${testRunEnv}"`, async () => {
+                            const parser = new TestParser({ testRunEnv });
+
+                            await loadFiles_({ parser, files: ["foo/bar", "baz/qux"] });
+
+                            assert.calledOnceWith(setupTransformHook, { removeNonJsImports });
+                        });
+                    });
+                });
+
                 it("should setup hook before read files", async () => {
                     await loadFiles_({ files: ["foo/bar", "baz/qux"] });
 

diff --git a/test/src/test-reader/test-transformer.ts b/test/src/test-reader/test-transformer.ts
@@ -59,18 +59,26 @@ describe("test-transformer", () => {
             });
         });
 
-        describe("should not transform", () => {
-            [".css", ".less", ".scss", ".jpg", ".png", ".woff"].forEach(extName => {
-                it(`asset with extension: "${extName}"`, () => {
-                    let transformedCode;
-                    const fileName = `some${extName}`;
-                    (pirates.addHook as SinonStub).callsFake(cb => {
-                        transformedCode = cb(`import "${fileName}"`, fileName);
-                    });
+        [true, false].forEach(removeNonJsImports => {
+            describe(`should ${removeNonJsImports ? "" : "not "}remove non-js imports`, () => {
+                [".css", ".less", ".scss", ".jpg", ".png", ".woff"].forEach(extName => {
+                    it(`asset with extension: "${extName}"`, () => {
+                        let transformedCode;
+                        const fileName = `some${extName}`;
+                        (pirates.addHook as SinonStub).callsFake(cb => {
+                            transformedCode = cb(`import "${fileName}"`, fileName);
+                        });
 
-                    setupTransformHook();
+                        setupTransformHook({ removeNonJsImports });
+
+                        const expectedCode = ['"use strict";'];
 
-                    assert.equal(transformedCode, '"use strict";');
+                        if (!removeNonJsImports) {
+                            expectedCode.push("", `require("some${extName}");`);
+                        }
+
+                        assert.equal(transformedCode, expectedCode.join("\n"));
+                    });
                 });
             });
         });