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

Feature request: Run multiple times programatically #1925

Closed
nicojs opened this issue Aug 3, 2021 · 1 comment · Fixed by #1934
Closed

Feature request: Run multiple times programatically #1925

nicojs opened this issue Aug 3, 2021 · 1 comment · Fixed by #1934

Comments

@nicojs
Copy link
Contributor

nicojs commented Aug 3, 2021

It would be great if Jasmine would allow users to run multiple times programmatically, without reloading the files.

Expected Behavior

Example:

// run-jasmine.js
const Jasmine = require('jasmine');

async function main() {
  const jasmine = new Jasmine({ projectBaseDir: process.cwd() });
  let specId = 'spec0';
  jasmine.loadConfigFile('./spec/support/jasmine.json');
  jasmine.env.configure({
    specFilter(sp) {
      return sp.id === specId;
    },
  });
  jasmine.exit = () => {};
  await jasmine.execute();
  specId = 'spec2';
  await jasmine.execute();
}

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

Expected:

$ node run-jasmine.js
Started
.


Ran 1 of 5 specs
1 spec, 0 failures
Finished in 0.015 seconds
Started
.


Ran 1 of 5 specs
1 spec, 0 failures
Finished in 0.015 seconds

Current Behavior

$ node run-jasmine.js
Started
.


Ran 1 of 5 specs
1 spec, 0 failures
Finished in 0.015 seconds
Started
F./node_modules/jasmine-core/lib/jasmine-core/jasmine.js:4253
          throw error;
          ^

TypeError: Cannot read property 'spies' of undefined
    at currentSpies (/home/nicojs/github/stryker-js/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1934:56)
    at SpyRegistry.clearSpies (/home/nicojs/github/stryker-js/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:8849:19)
    at clearResourcesForRunnable (/home/nicojs/github/stryker-js/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1439:19)
    at QueueRunner.onComplete (/home/nicojs/github/stryker-js/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1823:13)
    at Immediate.<anonymous> (/home/nicojs/github/stryker-js/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7814:12)

Possible Solution

I think jasmine has a similar feature as mocha's cleanReferencesAfterRun. So during spec execution, closures get deleted to prevent memory leaks in case of very big test scenarios.

I've implemented a feature to optionally disable this feature in Mocha. You can see an example here: mochajs/mocha#2783 (comment). I would like to do the same thing here.

Suite that reproduces the behavior (for bugs)

Just the normal jasmine init example is used to produce the error above.

Context

When would you want to run the same suite multiple times without reloading the files? One word: mutation testing (or, I guess 2 words?)

During mutation testing, a mutation testing framework, like StrykerJS, will rerun your suite multiple times while changing the active mutant (via a global variable) as well as which tests are filtered (using the specFilter feature).

We're currently working around it by clearing all files from the require.cache between runs. This has a negative impact on performance, especially in big projects where 1000's of runs are needed.

Not only that but the require-cache approach wont work at all in an ES module scenario, because there is no way to clear files from an import cache or something.

See stryker-mutator/stryker-js#2922 for a detailed description of the proposed way of working.

@sgravrock
Copy link
Member

sgravrock commented Aug 6, 2021

Some history: Jasmine 2.x used to kinda-sorta allow the suite to be executed multiple times. Which is to say that it wasn't supported but you could mostly get away with it as long as you didn't try to re-run after any sort of failure occurred. Jasmine 3.0 and later drops references to describe and it closures as soon as it's done with them, as you suspected. This was done because without it, problematic memory leaks were very common. In particular, object-oriented SPA frameworks tend to have highly cyclical object graphs, such that retaining a reference to any object can cause the entire application to be kept in memory. (Users could avoid that either by nulling everything out in an afterEach or by storing things on this rather than in closure variables. Not very many people did that back when it mattered and I'm sure it's even less common today.) All of which is to say that Jasmine's default behavior still needs to be to drop references to closures. And also that there's a fairly large class of test suites that might not be able to use your approach to mutation testing.

In addition to retaining closures, there's also a fair bit of mutable state in various places that would need to be reset. I know that Spec, Suite, and Env all store state related to the result of execution. So does QueueRunner, although hopefully those wouldn't be reused across attempts. There might be other touch points as well. Actually resetting all that state should be pretty easy. But finding everything that needs to be reset is going to take some careful work.

All that said, your rationale makes sense and I'd be willing to review a well-tested PR that adds support for multiple runs, provided that it's off by default.

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

Successfully merging a pull request may close this issue.

2 participants