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

Mocha can't run tests twice programmatically #2783

Closed
aDu opened this issue May 3, 2017 · 42 comments · Fixed by #4234
Closed

Mocha can't run tests twice programmatically #2783

aDu opened this issue May 3, 2017 · 42 comments · Fixed by #4234
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal

Comments

@aDu
Copy link

aDu commented May 3, 2017

Programmatically, I cannot run a Mocha test twice.

I.e. I can't do mocha.run() twice.

I looked at a similar issue (#995), however, the solutions are not optimal and/or not work.

For example, deleting the require cache is not a solution for me, as I need to use the require cache.

Is there a way to mocha.run() twice?

Thanks in advance!

@aDu
Copy link
Author

aDu commented May 3, 2017

Mmmm ><. I guess I will stick with my solution of spawning a child process for each mocha run; but I wanted to avoid this.

@KingRial
Copy link

KingRial commented May 8, 2017

I had the same problem.

Reading the similar issue #736, it seems it's all about cleaning the "require.cache" of the previously loaded spec's file

Here my current solution:

launchTests(){
  let _sTestDir = 'tests/specs';
  let _oMocha = new Mocha();
  fs.readdirSync( _sTestDir ).filter( function( oFile ){
    return ( oFile.substr( -3 ) === '.js' );
  }).forEach(function( sFile ){
    let _sPathSpec = path.join( path.resolve(), _sTestDir, sFile );
    // Resetting caches to be able to launch tests multiple times...
   delete require.cache[ _sPathSpec ];
   // Adding test
    _oMocha.addFile( _sPathSpec );
  });
  _oMocha.run();
}

@boneskull
Copy link
Contributor

does the "retry" functionality not work?

@boneskull boneskull added status: waiting for author waiting on response from OP - more information needed type: question support question labels May 9, 2017
@aDu aDu closed this as completed May 16, 2017
@aDu
Copy link
Author

aDu commented May 16, 2017

No, I don't think so for the context of rerunning an entire test suite again. @KingRial's solution seems like it would work.

My suggestion is to update the mocha code to allow mocha.run() to be executed more than once (or a similar solution).

@josiahruddell
Copy link

I ran into the same problem. Why was this issue closed? Making a new instance of Mocha, and adding files should behave deterministically without modifying the require cache. Seems like a bug.

@aDu
Copy link
Author

aDu commented May 17, 2017

@josiahruddell Good point, I agree.

@aDu aDu reopened this May 17, 2017
@ScottFreeCode ScottFreeCode added type: bug a defect, confirmed by a maintainer confirmed and removed status: waiting for author waiting on response from OP - more information needed type: question support question labels Sep 4, 2017
@jcjolley

This comment has been minimized.

3 similar comments
@ORESoftware
Copy link

+1

@rawb
Copy link

rawb commented Nov 7, 2017

+1

@jamesmcging
Copy link

+1

@colinbendell
Copy link

I've tested the selective delete of the require.cache with hundreds of parallel reruns. See my comment on #955. (this basically removes the test file from require.cache immediately after it is evaluated). The question is, why do you need a durable require.cache?

@nicojs
Copy link
Contributor

nicojs commented Feb 15, 2019

For people interested, we've built the re-run functionality in the Stryker plugin for mocha: https://github.com/stryker-mutator/stryker/blob/930a7c39952e09a7d3f913fd9423bf78f608ec17/packages/mocha-runner/src/MochaTestRunner.ts#L48-L68

The problem we now face is that we want to rerun mocha, without cleaning require cash or reloading the files. We want to call it "hot reload". So rerun tests, without changing source code or test code.

Basically, what we want to do is this:

const mocha = new Mocha(...);
mocha.addFile(...);
mocha.run(() => {
  mocha.run() => {
    console.log('ran twice');
  });
});

But right now, the second run always errors with: "Cannot read property 'call' of undefined" (which isn't helpful...)

The problem is that mocha.run has side effects. Namely: it cleans the test functions

here:

mocha/lib/runner.js

Lines 895 to 897 in 0dacd1f

this.on(constants.EVENT_SUITE_END, function(suite) {
suite.cleanReferences();
});

and here:

mocha/lib/suite.js

Lines 455 to 481 in 0dacd1f

Suite.prototype.cleanReferences = function cleanReferences() {
function cleanArrReferences(arr) {
for (var i = 0; i < arr.length; i++) {
delete arr[i].fn;
}
}
if (Array.isArray(this._beforeAll)) {
cleanArrReferences(this._beforeAll);
}
if (Array.isArray(this._beforeEach)) {
cleanArrReferences(this._beforeEach);
}
if (Array.isArray(this._afterAll)) {
cleanArrReferences(this._afterAll);
}
if (Array.isArray(this._afterEach)) {
cleanArrReferences(this._afterEach);
}
for (var i = 0; i < this.tests.length; i++) {
delete this.tests[i].fn;
}
};

I think mocha.run isn't the place to start listening for the suite end event and clean up after. This should be done in the mocha cli file itself, right?

I'm willing to create the PR if the maintainers agree with this.

We could also add an option for this to keep the API backward compatible.

@plroebuck
Copy link
Contributor

This is just one aspect.. there are several others required to really hope to reuse a Mocha instance.
Have something in mind, but post 6.0 release.

@plroebuck
Copy link
Contributor

Of note, the Mocha-6.0 release API includes Mocha#unloadFiles. It's one step towards eventual mocha reuse...

@nicojs
Copy link
Contributor

nicojs commented Feb 19, 2019

Actually... no...

To delete the require cache correctly, not only the test files but also the source files should be cleared. Mocha doesn't keep track of those files. This is what we need to do in Stryker right now (the purgeFiles method takes care of it).

Just to make sure we don't confuse 2 things: we actually want to reuse the same instance of mocha without clearing any file cache. Should I create a separate issue for that?

@boneskull boneskull added type: feature enhancement proposal and removed type: bug a defect, confirmed by a maintainer labels Feb 25, 2019
@boneskull
Copy link
Contributor

essentially, you'd need to ensure Mocha's inner state was completely reset. given there are bits of state everywhere, it's kind of a tall order at this point without some significant refactors. granted, having a single source of truth for state would be a great refactor, but...nontrivial.

@nicojs
Copy link
Contributor

nicojs commented Feb 26, 2019

you'd need to ensure Mocha's inner state was completely reset. given there are bits of state everywhere, it's kind of a tall order at this point

Could you give me a list of things you can think about at the top of your head, with pointers on how to solve it? We should be able to create some kind of state object and pass that along. We can make it opt-in, so functionality remains the same if you don't want to use it.

@nicojs
Copy link
Contributor

nicojs commented Apr 22, 2020

The PR is finished. @boneskull would you care to take a look?

Great integration tests you guys have! I broke mocha's watch functionality apparently. Never would have known without those awesome tests!

I choose in the end NOT to call it autoDispose, rather i've called it cleanReferencesAfterRun. My reasoning is that dispose() on the mocha instance should also remove any uncaughtException listeners remaining on the process, but I don't want to remove the listeners directly after a test run, since the previous runner should still handle them (it was one of the integration tests). That's why I instead only clean all references after each run. You can still dispose everything at once using the dispose method on a mocha instance.

boneskull added a commit that referenced this issue May 11, 2020
…loses #2783

* Add ability to run tests in a mocha instance multiple times

* Rename `autoDispsoe` to `cleanReferencesAfterRun`,

Rename since we cannot dispose the entire mocha instance after a test run. We should keep the process.on('uncaughtException') handlers in place in order to not break existing functionality.

* Allow `unloadFiles` to reset `referencesCleaned`.

* Complete rename of _cleanReferencesAfterRun

* Add integration test for running a suite multiple times

* improve api docs

* Docs: fix dead link

* Make sure tests run on older node versions

* Remove `.only` 😅

* Implement `process.listenerCount` in the browser

* Implement mocha states in a finite state machine

* Fix some small remarks

* Make integration tests more damp

* Keep `Runner` api backward compatible

* Unload files when disposed

* Runnable.reset should also reset `err` and `state`

* Also reset hooks

Co-authored-by: Christopher Hiller <[email protected]>
craigtaub pushed a commit that referenced this issue May 21, 2020
…loses #2783

* Add ability to run tests in a mocha instance multiple times

* Rename `autoDispsoe` to `cleanReferencesAfterRun`,

Rename since we cannot dispose the entire mocha instance after a test run. We should keep the process.on('uncaughtException') handlers in place in order to not break existing functionality.

* Allow `unloadFiles` to reset `referencesCleaned`.

* Complete rename of _cleanReferencesAfterRun

* Add integration test for running a suite multiple times

* improve api docs

* Docs: fix dead link

* Make sure tests run on older node versions

* Remove `.only` 😅

* Implement `process.listenerCount` in the browser

* Implement mocha states in a finite state machine

* Fix some small remarks

* Make integration tests more damp

* Keep `Runner` api backward compatible

* Unload files when disposed

* Runnable.reset should also reset `err` and `state`

* Also reset hooks

Co-authored-by: Christopher Hiller <[email protected]>
@RossVertizan
Copy link

Thank you @nicojs, this works perfectly for what I am trying to achieve (something similar to your need).

Quick question though, where do I set cleanReferencesAfterRun, at the moment, I have hacked it into runner.run(), but can I pass it as an option? I see it is set on the Mocha object, but can't see how I would pass it to Mocha.

Many thanks for any insights.

@nicojs
Copy link
Contributor

nicojs commented Feb 17, 2021

You can call it on the mocha object itself, using the programmatic interface: https://mochajs.org/api/mocha

@RossVertizan are you building a mutation testing framework by any chance? 😉

@RossVertizan
Copy link

RossVertizan commented Feb 18, 2021

Ahh OK, so I need to call the cleanReferencesAfterRun method on the Mocha object. It's a shame it can't be passed as a Mocha option.

@boneskull @craigtaub - is there possibility we could add this as a Mocha option?

@nicojs - actually no not building a mutation testing framework. I am building a constrained random testing engine for web and mobile app testing we call Vitaq. We have a concept of action/page and next allowable action/page, so that the user can define a graph/model of their website/app and the tool will walk through the graph executing the script (suite in Mocha parlance) associated with each page. We can define sequences or user paths through the model and set functional coverage targets for the sequences, each page individually and for the use of constrained random variables and we use AI to drive the functional coverage target to closure.

We can (and frequently do) have loops e.g. navigating back to the home page before going off somewhere else and we therefore need the ability to execute the suite multiple times and hence the need for your enhancement.

I am currently looking at making the tool work with the webdriverio infrastructure, using Mocha as the test framework. So I don't have direct access to the Mocha object (it's in the webdriverio-mocha interface code) and It would be nice to be able to pass cleanReferencesAfterRun as a mocha option. The passing of options to Mocha is already supported by webdriverio.

I should add that the approach I am looking at will also require the ability to select which suite to run next (based on the selection made by Vitaq) rather than simply iterating through the list of available suites. I have hacked together something that demonstrates the concept, but I need to look at this in more detail - next job on the list :)

@gilad-moskowitz
Copy link

Can somebody please provide the syntax needed to programmatically run tests twice? Currently, I am having an issue where I am creating a Mocha instance, setting the cleanReferencesAfterRun : false, and when I run the tests again I get a 'done() called multiple times' error as the first run through had tests that failed due to timeout. I am not calling 'done()' anywhere since I am writing the tests using async/await so I don't know how to resolve the issue. The only thing I can think of is disposing of the mocha instance that ran the first suite of tests and creating a new one for the second suite, but if I call mocha.unloadFiles() or mocha.dispose(), my process ends. Any advice would be greatly appreciated.

@RossVertizan
Copy link

@gilad-moskowitz how are you "setting the cleanReferencesAfterRun : false" on the Mocha instance?
There is a method implemented to do this (called 'cleanReferencesAfterRun'), so you should call that method, is that what you are doing?

@nicojs
Copy link
Contributor

nicojs commented Jul 23, 2021

For anyone interested in this API, here is a small example:

const Mocha = require('mocha');

async function main(){
    const mocha = new Mocha();
    mocha.addFile('test/test.spec.js');
    mocha.cleanReferencesAfterRun(false); // => important! This allows multiple runs.
    await mocha.loadFilesAsync();
    mocha.run(() => {
        console.log('✅ done');
        mocha.run(() => {
            console.log('\t✅ done x2');
            mocha.dispose(); // => important if you want to keep the node process alive, otherwise memory leak 🕳
        });
    });
}

main().catch(err => {
    console.error(err);
    process.exitCode = 1;
});

This also works with async specs, see my test.spec.js:

function sleep(n) {
    return new Promise(res => setTimeout(res, n));
}

it('should work', () => {})

it('should work async as well', () => sleep(100) );

@RossVertizan
Copy link

@nicojs Thanks - that's what I should have done.

@gilad-moskowitz
Copy link

Hello, thank you so much for the quick response. So, my code that returns the done() called multiple times error is as follows:

const Mocha = require('mocha');
const fs = require('fs')
const path = require('path')

async function main(){
    const mocha = new Mocha({
         ui: 'bdd',
         reporter: 'custom.js',
    });
    fs.readdirSync('test').filter(function(file) {
         return file.substr(-8) === '.test.js';
    }).forEach(function(file){
         mocha.addFile(path.join('test', file));
    });
    }
    await mocha.loadFilesAsync();
    mocha.cleanReferencesAfterRun(false).run(() => {
        baseTimeout = 2*baseTimeout;
        mocha.cleanReferencesAfterRun(true).run(() => {mocha.dispose();});
    });
}

main().catch(err => {
    console.error(err);
    process.exitCode = 1;
});

I set the timeout for each test as being equal to the baseTimeout before each test. So on the first run the tests have a timeout equal to the baseTimeout and on the second run the tests have a timeout equal to 2*baseTimeout. I use Chai for making API requests and all my tests are written as (for example):

describe('Suite name', function (){
    it('Test name', async function(){
         let result = await chai.request(API).get('...');
         await expect(result.status).to.equal(200);
    });
})

On the first run, sometimes the tests fail because the timeout. This causes tests on the second run to error with done() called multiple times even though all my code uses ASYNC/AWAIT so there is no done() call at all.

My guess as to why this issue occurs is because the chai request that I am awaiting doesn't resolve until my second run and when it resolves it causes a problem.

@gilad-moskowitz
Copy link

@nicojs @RossVertizan Just wanted to follow up and see if you had a chance to look at my most recent comment. I haven't found any work arounds to the issue so any advice would be greatly appreciated.

@nicojs
Copy link
Contributor

nicojs commented Aug 24, 2021

Feel free to upload a small reproduction repo, so I can try to reproduce this locally

@gilad-moskowitz
Copy link

gilad-moskowitz commented Aug 25, 2021

@nicojs I created a private reproduction repository (edit: and sent you an invite as a collaborator). You should be able to recreate it locally using the files there. Thank you so much for the help.

@gilad-moskowitz
Copy link

@nicojs Just wanted to follow up again and check to see if you had gotten the access to my reproduction repo. Thank you

@gabssnake
Copy link

When creating multiple instances of Mocha it will leak EventListeners attached to process uncaughtException.

This happens even if you don't manually set the cleanReferencesAfterRun option.

Calling mocha.dispose() solves the issue for me.

kanej added a commit to NomicFoundation/hardhat that referenced this issue Dec 8, 2021
This is a mocha caching and cleanup issue addressed here: mochajs/mocha#2783.

Relates to #1720
kanej added a commit to NomicFoundation/hardhat that referenced this issue Dec 10, 2021
This is a mocha caching and cleanup issue addressed here: mochajs/mocha#2783.

Multiple runs of the test task within a hardhat script are now enabled. The resolution is `mocha.dispose` at the end of a run to clear up state.

The mocha dispose method was added in [email protected] but the @types/mocha doesn't reflect it till `9.0.0`. This commit:

* bumps mocha as a dep in hardhat-core to 7.2.0 to be explicit
* bumps mocha as a dev-dep everywhere else to 7.2.0 for consistency in our local dev environment
* bumps @types/mocha to 9 in dev-deps in all packages to allow use of dispose and consistency across packages - no code changes are required to support this. Note that the sample typescript project already provides a default @types/mocha on version 9.0.0

Relates to #1720
kanej added a commit to NomicFoundation/hardhat that referenced this issue Dec 15, 2021
Fix reruns of `test` in scripts. This is a mocha caching and cleanup issue addressed here: mochajs/mocha#2783.

Multiple runs of the test task within a hardhat script are now enabled. The resolution is `mocha.dispose` at the end of a run to clear up state.

The mocha dispose method was added in [email protected] but the @types/mocha doesn't reflect it till `9.0.0`. This commit:

* bumps mocha as a dep in hardhat-core to 7.2.0 to be explicit
* bumps mocha as a dev-dep everywhere else to 7.2.0 for consistency in our local dev environment
* bumps @types/mocha to 9 in dev-deps in all packages to allow use of dispose and consistency across packages - no code changes are required to support this. Note that the sample typescript project already provides a default @types/mocha on version 9.0.0

Relates to #1720
alcuadrado pushed a commit to NomicFoundation/hardhat-waffle that referenced this issue Aug 11, 2022
Fix reruns of `test` in scripts. This is a mocha caching and cleanup issue addressed here: mochajs/mocha#2783.

Multiple runs of the test task within a hardhat script are now enabled. The resolution is `mocha.dispose` at the end of a run to clear up state.

The mocha dispose method was added in [email protected] but the @types/mocha doesn't reflect it till `9.0.0`. This commit:

* bumps mocha as a dep in hardhat-core to 7.2.0 to be explicit
* bumps mocha as a dev-dep everywhere else to 7.2.0 for consistency in our local dev environment
* bumps @types/mocha to 9 in dev-deps in all packages to allow use of dispose and consistency across packages - no code changes are required to support this. Note that the sample typescript project already provides a default @types/mocha on version 9.0.0

Relates to #1720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.