Mocha: afterEach cleanup running is late

[roy-codefresh]

When I using mocha to test code with printing I need to mock it in the beforeEach and restore the original printing in the afterEach hook. But in mocha the afterEach hook is running after the pass and the end test emitted to the reporters meaning the printing of the pass check or the fail check is printed into my mock function.

As an exmple code:

'use strict';

describe('Hello World', function () {

    beforeEach(function () {
        this.originalConsoleLog = console.log;

        console.log = text => {
            this.loggedText = text;
        };
    });

    afterEach(function () {
        console.log = this.originalConsoleLog;
    });

    it('passing test', function () {
        console.log('hello world !!');

        if (this.loggedText !== 'hello world !!') {
            throw new Error('Did not print hello world');
        }
    });

    it('failing test', function () {
        console.log('Hello');

        if (this.loggedText !== 'hello world !!') {
            throw new Error('Did not print hello world');
        }
    });
});

When running this with no flags the output is:

  Hello World


  1 passing (8ms)
  1 failing

  1) Hello World failing test:
     Error: Did not print hello world
      at Context.<anonymous> (test.spec.js:29:19)

I would have expected that the output would be like this:

  Hello World
    ✓ passing test
    1) failing test

  1 passing (8ms)
  1 failing

  1) Hello World failing test:
     Error: Did not print hello world
      at Context.<anonymous> (test.spec.js:29:19)

[ScottFreeCode]

Alternatively, sidestepping the question of whether test results should be emitted to the reporter before or after that test's hooks (and the potential complication of changing it, especially considering that tests can have multiple hooks and that all such things are complicated by nested suites), what if there was a flag to make the reporters save the references to the non-stubbed output functions when they're first loaded, so that they can't be affected by this?

[Munter]

Is there ever a reason where you would not want the reporter to be able to write it's report?

I think saving a reference to the original output function is a great idea.

Before we have that though, you can use a promise chain inside your test to remove the mocks before the test resolves

[roy-codefresh]

Saving the original output function is great idea, but It's not enough.

Like when a developer wants to mock the process.stdout.write function. In this case the function console.log would remain the original but still won't work because it's using the stdout.write function.

This is way I think that the afterEach hooks should runs as soon as possible, because everything happens before the cleanup might be compromised from the tests mocks.

[ScottFreeCode]

Is there ever a reason where you would not want the reporter to be able to write it's report?

I could imagine somebody using Mocha through the programmatic API but wanting to redirect the reporter output and thus monkey-patching console.log and/or process.stdout specifically to capture reporter output. I don't know if anyone's actually doing this (but Mocha has a lot of users and we know some people have been doing stranger things...), or whether there's a better way to accomplish it (which could make a big difference re. supporting it with flags vs. just getting rid of it in a major release), but conservatively it'd be nice to avoid breaking it for them if that doesn't turn out to be too much trouble. (I haven't forgotten the hubbub over making done+promises an error even though it was in a major release.) We could even reverse the flag in a major release if we haven't found a better alternative by then, making immunity to monkey-patching be the default but allowing users to opt into the old behavior.

Like when a developer wants to mock the process.stdout.write function. In this case the function console.log would remain the original but still won't work because it's using the stdout.write function.

But reporters could save the standard output functions also (as far as I'm aware they're not using any special functionality of console.log); in principle it should work, it's just a question of which functions need to be saved.

Whereas, in contrast, I don't even know how to begin finding out what could be broken if we start changing the order of code execution vs. events emitted to the reporter. Keep in mind Mocha supports custom reporters, multiple afterEach hooks, after hooks that could just as well be cleaning stuff up instead of afterEach ones, and nested suites where the cleanup might be associated with an outer suite instead of the one in which the test is running (and we've already got other bugs relating to when hooks are run in combinations of nested suites). Deviating from direct correlation of events to the actual test and hook runs* not only is likely to make future users (or even future maintainers) wonder why it isn't simpler, but also opens up a can of worms as to how to get the behavior correct -- and at a glance I'm not even sure just how big that can of worms would be. So if there's a simple and straightforward alternative where we don't have to decide where to draw the line for delaying the events or how to handle edge cases, I think it should be explored -- just my two cents' worth.

Obviously, someone (whether a current maintainer/contributor or not) has to try to implement one or the other solution or else this discussion is just academic. It should definitely be fixed, one way or the other. And I could very well change my mind as I dig into some actual code (it wouldn't be the first time, heh heh); I just want to be clear, before we get too far, about what we'll need to address in each option.

Tangentially, the reporters are already saving the original references to other global elements that get replaced with mocks.

*I don't think anything's running late as is or even not already running as soon as possible; as far as I'm aware it's more like the test reporter events are considered part of running the test. Maybe that's a matter of perspective, but if perspective helps...

[ScottFreeCode]

(Sorry for being long-winded; I don't have time right at this moment to edit my explanation down as much as I know I ought to.)

[ORESoftware]

http://www.goodreads.com/quotes/21422-i-didn-t-have-time-to-write-a-short-letter-so