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

Setup for multiple test files #144

Open
Xerillio opened this issue Mar 13, 2023 · 4 comments
Open

Setup for multiple test files #144

Xerillio opened this issue Mar 13, 2023 · 4 comments

Comments

@Xerillio
Copy link

I started using this library for a single test file using Mocha to run the tests with mocha ./bin/tests/**/*.test.js - all good. Then I wanted to add tests in another file and suddenly I got an error message similar to the one described in #96 :

Cannot read properties of undefined (reading 'filename')

The README suggests putting the rewiring in a common file that all tests can use instead of a per-test-file rewiring, so this was my rewire.ts:

import rewiremock from "rewiremock";
import * as sinon from "sinon";

export const testInputs = new Map<string, any>();
export const testVariables = new Map<string, any>();

rewiremock("azure-pipelines-task-lib")
    .with({
        getInput: sinon.stub().callsFake((i: string) => testInputs.get(i)),
        getInputRequired: sinon.stub().callsFake((i: string) => testInputs.get(i)),
        getBoolInput: sinon.stub().callsFake((i: string) => testInputs.get(i)),
        getVariable: sinon.stub().callsFake((v: string) => testVariables.get(v))
    });

rewiremock.overrideEntryPoint(module); // Removing this line doesn't seem to change anything
rewiremock.enable();

export { rewiremock };

This starts failing even if I add a new empty *.test.ts file, while having only one *.test.ts file works fine. Perhaps this is a duplicate of #96, but if not, how is it recommend to set up a test project with multiple test files?

What I have now as a workaround for the above issue:

// inputs.test.ts
describe("Inputs", () => {
    before(() => {
        rewiremock("azure-pipelines-task-lib")
            .with({
                getInput: sinon.stub().callsFake((i: string) => testInputs.get(i)),
                // ...
            });
    });

    after(() => {
        rewiremock.clear();
    });

    // I'm testing my `Inputs` class
    const createSut = async(): Promise<Inputs> => {
        rewiremock.enable();
        const InputsConstructor = (await import("../src/inputs")).Inputs;
        rewiremock.disable();
        return new InputsConstructor();
    };

    beforeEach(() => {
        testInputs.clear();
        // Set default `testInputs` values...
        testInputs.set(fileGlobInput, "test glob");
    });

    it("should return the specified task inputs", async() => {
        const sut = await createSut();

        expect(sut.fileGlob).to.equal("test glob");
        // ...
    });
});

This boilerplate with before, after and createSut would be duplicated in each test file.

@theKashey
Copy link
Owner

Cannot read properties of undefined (reading 'filename')

Can you please provide a more complete callstack.


What would change if you do rewiremock.overrideEntryPoint(module); before anything else?

In any case there is no recommendation to extract mocking of one file to a "setup" file, it was only to setup your unique configuration if present.
Would you consider automocking as a solution for azure-pipelines-task-lib?

Xerillio added a commit to Xerillio/azure-devops-pr-commentator that referenced this issue Mar 13, 2023
* Changed `rewire.ts` to provide helper methods instead of rewiring
  directly in the global scope which seemed to cause issues. See
  theKashey/rewiremock#144.
* Added NPM package `nyc` to provide coverage reports. Use
  `npm run coverage`.
* Improved coverage of `Inputs`.
@Xerillio
Copy link
Author

@theKashey, here's the stacktrace:

TypeError: Cannot read properties of undefined (reading 'filename')
    at getModuleName (F:\coding\repos\azure-devops-pr-commentator\node_modules\rewiremock\lib\utils\modules.js:10:24)
    at Function.mockLoader [as _load] (F:\coding\repos\azure-devops-pr-commentator\node_modules\rewiremock\lib\executor.js:275:84)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:170:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:541:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)
    at async formattedImport (F:\coding\repos\azure-devops-pr-commentator\node_modules\mocha\lib\nodejs\esm-utils.js:9:14)
    at async exports.requireOrImport (F:\coding\repos\azure-devops-pr-commentator\node_modules\mocha\lib\nodejs\esm-utils.js:42:28)
    at async exports.loadFilesAsync (F:\coding\repos\azure-devops-pr-commentator\node_modules\mocha\lib\nodejs\esm-utils.js:100:20)
    at async singleRun (F:\coding\repos\azure-devops-pr-commentator\node_modules\mocha\lib\cli\run-helpers.js:125:3)
    at async exports.handler (F:\coding\repos\azure-devops-pr-commentator\node_modules\mocha\lib\cli\run.js:370:5)

Moving the overrideEntryPoint call up produces a completely identical stacktrace:

import rewiremock from "rewiremock";
import * as sinon from "sinon";

rewiremock.overrideEntryPoint(module);

export const testInputs = new Map<string, any>();

rewiremock("azure-pipelines-task-lib")
    .with({
        getInput: sinon.stub().callsFake((i: string) => testInputs.get(i)),
        getInputRequired: sinon.stub().callsFake((i: string) => testInputs.get(i)),
        getBoolInput: sinon.stub().callsFake((i: string) => testInputs.get(i))
    });
rewiremock.enable();

I'll definitely take a look at automocking, thanks for suggesting that - I found the README a bit overwhelming so I didn't see that option 😅

@Xerillio
Copy link
Author

@theKashey Just to make sure I'm not getting it wrong. If I want to mock a node module it currently doesn't work with automocking because of #121, right? Since it only looks for the __mocks__ folder inside a subfolder of node_modules - at least that's what my attempt at using that approach shows.

@theKashey
Copy link
Owner

🤦 you are absolutely correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants