Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ability to load local files through cli option "require" #797

Merged
merged 1 commit into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const info = require("./info");
const { Hermione } = require("../hermione");
const pkg = require("../../package.json");
const logger = require("../utils/logger");
const { requireModule } = require("../utils/module");

let hermione;

Expand Down Expand Up @@ -58,7 +59,7 @@ exports.run = () => {
.arguments("[paths...]")
.action(async paths => {
try {
handleRequires(program.require);
await handleRequires(program.require);

const isTestsSuccess = await hermione.run(paths, {
reporters: program.reporter || defaults.reporters,
Expand Down Expand Up @@ -99,10 +100,6 @@ function preparseOption(program, option) {
return configFileParser[option];
}

function handleRequires(requires = []) {
requires.forEach(module => require(module));
}

function compileGrep(grep) {
try {
return new RegExp(`(${grep})|(${escapeRe(grep)})`);
Expand All @@ -111,3 +108,9 @@ function compileGrep(grep) {
return new RegExp(escapeRe(grep));
}
}

async function handleRequires(requires = []) {
for (const modulePath of requires) {
await requireModule(modulePath);
}
}
10 changes: 10 additions & 0 deletions src/utils/fs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import fs from "fs";

export const exists = async (path: string): Promise<boolean> => {
try {
await fs.promises.access(path);
return true;
} catch (_) {
return false;
}
};
8 changes: 8 additions & 0 deletions src/utils/module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import path from "path";
import { exists } from "./fs";

export const requireModule = async <T = unknown>(modulePath: string): Promise<T> => {
const isModuleLocal = await exists(modulePath);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если переданный путь существует на fs, то с помощью path.resolve привожу к абсолютному пути и реквайрю. Такой подход подсмотрел в mocha


return require(isModuleLocal ? path.resolve(modulePath) : modulePath);
};
13 changes: 10 additions & 3 deletions src/worker/hermione-facade.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const Promise = require("bluebird");
const debug = require("debug")(`hermione:worker:${process.pid}`);
const ipc = require("../utils/ipc");
const { MASTER_INIT, MASTER_SYNC_CONFIG, WORKER_INIT, WORKER_SYNC_CONFIG } = require("../constants/process-messages");
const { requireModule } = require("../utils/module");

module.exports = class HermioneFacade {
static create() {
Expand Down Expand Up @@ -45,15 +46,21 @@ module.exports = class HermioneFacade {

ipc.on(MASTER_INIT, ({ configPath, runtimeConfig } = {}) => {
try {
const promise = Promise.resolve();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Подписка на событие - не асинхронная операция, поэтому реализовал через докидывание промисов.


if (runtimeConfig.requireModules) {
runtimeConfig.requireModules.forEach(module => require(module));
runtimeConfig.requireModules.forEach(modulePath => {
promise.then(requireModule(modulePath));
});
}

RuntimeConfig.getInstance().extend(runtimeConfig);
const hermione = Hermione.create(configPath);

debug("worker initialized");
resolve(hermione);
promise.then(() => {
debug("worker initialized");
resolve(hermione);
});
} catch (e) {
debug("worker initialization failed");
reject(e);
Expand Down
10 changes: 6 additions & 4 deletions test/src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ describe("cli", () => {
});

it('should require modules specified in "require" option', async () => {
const fooRequire = sandbox.stub().returns({});
const requireModule = sandbox.stub();
const stubHermioneCli = proxyquire("src/cli", {
foo: (() => fooRequire())(),
"../utils/module": { requireModule },
});

await run_("--require foo", stubHermioneCli);

assert.calledOnce(fooRequire);
assert.calledOnceWith(requireModule, "foo");
});

it("should create Hermione without config by default", async () => {
Expand Down Expand Up @@ -155,7 +155,9 @@ describe("cli", () => {
});

it("should use require modules from cli", async () => {
const stubHermioneCli = proxyquire("src/cli", { foo: {} });
const stubHermioneCli = proxyquire("src/cli", {
"../utils/module": { requireModule: sandbox.stub() },
});
await run_("--require foo", stubHermioneCli);

assert.calledWithMatch(Hermione.prototype.run, any, { requireModules: ["foo"] });
Expand Down
33 changes: 33 additions & 0 deletions test/src/utils/fs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import fs from "fs";
import sinon, { SinonStub } from "sinon";
import { exists } from "../../../src/utils/fs";

describe("utils/fs", () => {
const sandbox = sinon.createSandbox();

beforeEach(() => {
sandbox.stub(fs.promises, "access");
});

afterEach(() => sandbox.restore());

describe("exists", () => {
it("should return 'true' if file exists", async () => {
const path = "./some-path.js";
(fs.promises.access as SinonStub).withArgs(path).resolves();

const isExists = await exists(path);

assert.isTrue(isExists);
});

it("should return 'false' if file doesn't exists", async () => {
const path = "./some-path.js";
(fs.promises.access as SinonStub).withArgs(path).rejects(new Error("o.O"));

const isExists = await exists(path);

assert.isFalse(isExists);
});
});
});
52 changes: 52 additions & 0 deletions test/src/utils/module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import path from "path";
import sinon, { SinonStub } from "sinon";
import proxyquire from "proxyquire";
import { requireModule as realRequireModule } from "../../../src/utils/module";

describe("utils/module", () => {
let isNodeModuleRequired: SinonStub;
let isLocalModuleRequired: SinonStub;
let exists: SinonStub;
let requireModule: typeof realRequireModule;

const sandbox = sinon.createSandbox();
const nodeModulePath = "foo-module";
const relativeLocalModulePath = "./bar-module";
const absoluteLocalModulePath = path.resolve(relativeLocalModulePath);

beforeEach(() => {
isNodeModuleRequired = sinon.stub();
isLocalModuleRequired = sinon.stub();
exists = sinon.stub();

({ requireModule } = proxyquire.noCallThru().load("../../../src/utils/module", {
"./fs": { exists },
[nodeModulePath]: isNodeModuleRequired,
[absoluteLocalModulePath]: isLocalModuleRequired,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не придумал как это по другому протестить. require нормально застабать невозможно (я не смог по крайней мере).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай не городить такое. здесь минимальная фикстура явно будет смотреться в разы лучше и проще

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Что имеешь ввиду под минимальной фикстурой? У меня это вроде бы тоже минимальная. Т.е. проверяю правильный require модуля из node_modules и локального модуля.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ну ты здесь всё застабал и от реальной реализации осталась только структура твоих модулей. Я вообще сомневаюсь, что этот тест что-то отловит, если ты что-то в логике поменяешь :)

Я имел в виду то, что вместо того, чтобы стабать реквайр, можно его выполнить по-настоящему. Да и модуль можно в виде пустышки положить, чтобы его зареквайрить вживую

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ну ты здесь всё застабал и от реальной реализации осталась только структура твоих модулей

ну так это же unit-тесты. Я в них застабал все лишнее. И проверяю только логику самой функции. Т.е. корректный реквайр модуля из node_modules или реквайр файл по абсолютному пути.

Я вообще сомневаюсь, что этот тест что-то отловит, если ты что-то в логике поменяешь :)

Если логику изменить, то тест упадет. Я это проверял при написании.

Я имел в виду то, что вместо того, чтобы стабать реквайр, можно его выполнить по-настоящему. Да и модуль можно в виде пустышки положить, чтобы его зареквайрить вживую

Ну по факту я не стабаю require. Я говорю, что при require такого несуществующего модуля вызови мой стаб и дальше я смотрю был он вызван или нет.

Про создание модуля на fs и его require я тоже думал. Но непонятно как в этом кейсе выполнить ассерт и убедится, что было зареквайрено то что нужно. Поэтому использовал текущий подход.

}));
});

afterEach(() => sandbox.restore());

describe("requireModule", () => {
it("should require module from node-modules", async () => {
exists.withArgs(nodeModulePath).resolves(false);
const module = await requireModule<SinonStub>(nodeModulePath);

module();

assert.calledOnce(isNodeModuleRequired);
assert.notCalled(isLocalModuleRequired);
});

it("should require module from node-modules", async () => {
exists.withArgs(relativeLocalModulePath).resolves(true);
const module = await requireModule<SinonStub>(relativeLocalModulePath);

module();

assert.calledOnce(isLocalModuleRequired);
assert.notCalled(isNodeModuleRequired);
});
});
});
13 changes: 7 additions & 6 deletions test/src/worker/hermione-facade.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"use strict";

const proxyquire = require("proxyquire");
const { AsyncEmitter } = require("src/events/async-emitter");
const { Hermione } = require("src/worker/hermione");
const { makeConfigStub } = require("../../utils");
Expand Down Expand Up @@ -46,18 +47,18 @@ describe("worker/hermione-facade", () => {
});

it("should require passed modules", async () => {
const hermioneFacadeModule = module.children.find(({ filename }) =>
/\/hermione-facade\.js$/.test(filename),
);
sandbox.stub(hermioneFacadeModule, "require");
const requireModule = sinon.stub();
const HermioneFacadeModule = proxyquire("src/worker/hermione-facade", {
"../utils/module": { requireModule },
});

ipc.on.withArgs(MASTER_INIT).yieldsAsync({
runtimeConfig: { requireModules: ["foo"] },
});

await hermioneFacade.init();
await HermioneFacadeModule.create().init();

assert.calledOnceWith(hermioneFacadeModule.require, "foo");
assert.calledOnceWith(requireModule, "foo");
});
});

Expand Down
Loading