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 programmatic API doesn't report retries #2188

Closed
cletusw opened this issue Apr 5, 2016 · 14 comments · Fixed by #4181
Closed

Mocha programmatic API doesn't report retries #2188

cletusw opened this issue Apr 5, 2016 · 14 comments · Fixed by #4181
Labels
type: bug a defect, confirmed by a maintainer type: feature enhancement proposal

Comments

@cletusw
Copy link

cletusw commented Apr 5, 2016

When using Mocha programmatically with the new retries feature, tests that fail the first time but eventually succeed are not marked as "passed" in the suite's tests array.

index.js:

var Mocha = require('mocha');
var mocha = new Mocha({
  retries: 1
});
mocha.addFile('test.js');
mocha.run().on('suite end', function(suite) {
  console.dir(suite);
});

test.js:

var currentRun = 0;

it('should mark tests that succeed on 2nd try as "passed"', function() {
  currentRun++;
  console.log('running test');
  if (currentRun === 1) {
    console.log('failing test');
    throw new Error('hi');
  }
  console.log('passing test');
});

Output:

running test
failing test
running test
passing test
  ✓ should mark tests that succeed on 2nd try as "passed"
Suite {
  title: '',
  ctx: 
   Context {
     _runnable: 
      Test {
        title: 'should mark tests that succeed on 2nd try as "passed"',
        fn: [Function],
        async: 0,
        sync: true,
        _timeout: 2000,
        _slow: 75,
        _enableTimeouts: true,
        timedOut: false,
        _trace: [Error: done() called multiple times],
        _retries: 1,
        _currentRetry: 1,
        pending: false,
        type: 'test',
        body: 'function () {\n  currentRun++;\n  console.log(\'running test\');\n  if (currentRun === 1) {\n    console.log(\'failing test\');\n    throw new Error(\'hi\');\n  }\n  console.log(\'passing test\');\n}',
        _allowedGlobals: undefined,
        parent: [Circular],
        file: '/Users/claytonwatts/dev/domo/domoweb/DomoWeb/test.js',
        ctx: [Circular],
        _events: [Object],
        _eventsCount: 1,
        callback: [Function: done],
        duration: 0,
        state: 'passed',
        speed: 'fast' },
     test: 
      Test {
        title: 'should mark tests that succeed on 2nd try as "passed"',
        fn: [Function],
        async: 0,
        sync: true,
        _timeout: 2000,
        _slow: 75,
        _enableTimeouts: true,
        timedOut: false,
        _trace: [Error: done() called multiple times],
        _retries: 1,
        _currentRetry: 1,
        pending: false,
        type: 'test',
        body: 'function () {\n  currentRun++;\n  console.log(\'running test\');\n  if (currentRun === 1) {\n    console.log(\'failing test\');\n    throw new Error(\'hi\');\n  }\n  console.log(\'passing test\');\n}',
        _allowedGlobals: undefined,
        parent: [Circular],
        file: '/Users/claytonwatts/dev/domo/domoweb/DomoWeb/test.js',
        ctx: [Circular],
        _events: [Object],
        _eventsCount: 1,
        callback: [Function: done],
        duration: 0,
        state: 'passed',
        speed: 'fast' } },
  suites: [],
  tests: 
   [ Test {
       title: 'should mark tests that succeed on 2nd try as "passed"',
       async: 0,
       sync: true,
       _timeout: 2000,
       _slow: 75,
       _enableTimeouts: true,
       timedOut: false,
       _trace: [Error: done() called multiple times],
       _retries: 1,
       _currentRetry: 0,
       pending: false,
       type: 'test',
       body: 'function () {\n  currentRun++;\n  console.log(\'running test\');\n  if (currentRun === 1) {\n    console.log(\'failing test\');\n    throw new Error(\'hi\');\n  }\n  console.log(\'passing test\');\n}',
       file: '/Users/claytonwatts/dev/domo/domoweb/DomoWeb/test.js',
       parent: [Circular],
       ctx: [Object],
       _events: [Object],
       _eventsCount: 1,
       callback: [Function: done],
       duration: 0 } ],
  pending: false,
  _beforeEach: [],
  _beforeAll: [],
  _afterEach: [],
  _afterAll: [],
  root: true,
  _timeout: 2000,
  _enableTimeouts: true,
  _slow: 75,
  _bail: undefined,
  _retries: 1,
  delayed: false,
  _events: { 'pre-require': [ [Function], [Function] ] },
  _eventsCount: 1 }

  1 passing (22ms)

Notice that the test objects in Context are correctly marked with state: "passed", but the one in the tests array is from the first run (note _currentRetry: 0) and has no state property.

@cletusw
Copy link
Author

cletusw commented Apr 5, 2016

@longlho

@justspamjustin
Copy link

+1 The retry feature is great. We just need it supported in the programmatic API as well.

@longlho
Copy link
Contributor

longlho commented Apr 6, 2016

kk I'll try to take a look asap :)

@longlho
Copy link
Contributor

longlho commented Apr 12, 2016

@cletusw @justspamjustin So every time runner runs a suite, it shallow clones the tests, which I'm not entirely sure whether it should be deep clone instead. The reason is that tests in suite are essentially input, and shallow clone during runnable doesn't prevent runner from modifying input inline, which I think can cause side effects (in this case all test results are persisted in the input test object). All public reporters hook into the event lifecycle, which received the "output" test as a param, thus working correctly w/ retries.

In your scenario, since retries clone the tests, the original test that was passed in (or globbed by Mocha, for that matter) isn't considered passed, bc actually it did not, the 2nd cloned one did (which is by design). I think the solution for u guys might be to follow the reporter pattern of listening to certain events since it's more of like a "public" API.

cc @danielstjules @boneskull for discussion.

@cletusw
Copy link
Author

cletusw commented Apr 12, 2016

So does that mean we can't use the data in the "suite end" event?

On Mon, Apr 11, 2016, 6:35 PM Long Ho notifications@github.com wrote:

@cletusw https://github.com/cletusw @justspamjustin
https://github.com/justspamjustin So every time runner is run, it
shallow clone the tests, which I'm not entirely sure whether it should be
deep clone instead. The reason is that tests in suite are essentially
input, and shallow clone during runnable doesn't prevent runner from
modifying input inline, which I think can cause side effects (in this
case all test results are persisted in the test object). All public
reporters hook into the event lifecycle, which received the cloned test
as a param, thus working correctly w/ retries.

In your scenario, since retries clone the tests, the original test that
was passed in (or globbed by Mocha, for that matter) isn't considered
passed, bc actually it did not, the 2nd cloned one did (which is by
design). I think the solution for u guys might be to follow the reporter
pattern of listening to certain events since it's more of like a "public"
API.

cc @danielstjules https://github.com/danielstjules @boneskull
https://github.com/boneskull for discussion.


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#2188 (comment)

@longlho
Copy link
Contributor

longlho commented Apr 12, 2016

that I'm not sure, @danielstjules @boneskull ? maybe have Suite store the list of "output" test results?

@cletusw
Copy link
Author

cletusw commented Apr 12, 2016

@longlho It seems to me that if we can externally listen to all the pass, fail, and pending events and compile a correct list, then mocha should be able to do the same internally and give correct data on a suite end event.

@cletusw
Copy link
Author

cletusw commented Apr 12, 2016

@longlho Thanks for looking into this, btw. retry for our end-to-end tests is something we've been wanting for a long time, and you've already gotten us most of the way there.

@longlho
Copy link
Contributor

longlho commented Apr 12, 2016

sure np :) I can certainly put in a PR of some sort to address this issue (which I think is either deep cloning the tests and/or store test results separately in the Suite). Kinda waiting for @danielstjules & @boneskull to chime in.

@cletusw
Copy link
Author

cletusw commented Apr 26, 2016

@danielstjules @boneskull Any update on this? This makes the data provided to the suite end event useless when using retries.

@longlho
Copy link
Contributor

longlho commented Apr 27, 2016

@cletusw based on this PR I think listening to pass/fail might yield better results

@stale
Copy link

stale bot commented Oct 17, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot added the stale this has been inactive for a while... label Oct 17, 2017
@boneskull boneskull removed the stale this has been inactive for a while... label Oct 17, 2017
@boneskull
Copy link
Member

not stale

@whxaxes
Copy link

whxaxes commented Apr 27, 2019

any progress ??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer type: feature enhancement proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants