🚀 Feature: Retry if afterEach fails

[aaronabramov]

in continuation of #2120 and #1989

the issue is that if anything throws in afterEach hook, the test won't retry and fail immediately.

I'll try to explain the use case.

When we test our application end-to-end (using webdriver or similar technologies) we need retries, because most of the time tests fail due to network errors, webdriver errors or other stuff that we don't control. And it's been working great if something sketchy happens inside the it body. But there is one use case:

When we test an app using webdriver, often times we want to make sure that there was no errors in the console after the test has finished. Even though the functionality works, something can throw (React.propTypes validation, logging, random undefined is not a function, etc..) and you most likely want to catch errors like that.
here is a simplified version of an example test:

describe('a button', function() {
    it('shows a modal window when button is clicked', function(done) {
        webdriverAPI.click('.button') // we find some button on our page that has a specific class and click it
        .then(() => webdriverAPI.isPresentOnThePage('.modal-window')) // then we expect a window to show up
        .then(() => done()); // if it's there, we assume that functionality is ok and pass the test
    });

    // in after each hook, we want to make sure nothing threw an error in the browser.
    // 
    // Just to be clear. the browser environment is a completely separate process and
    // not the current node.js process. We only interact with it via `webdriverAPI`
    afterEach(function(done) {
        webdriverAPI.getConsoleOutput().then((output) => {  // capture the output of the browser console that was printed during the test execution
            if (output) {
                // if there is something => blow up the test
                return done(new Error(`unexpected output in the console: ${output}`));
            }

            done();
        });
    });
});

It's been working fine, but sometimes you get a weird errors like profile_picture.jpg failed to load because of some network problem which brings your entire test suite down.
It would be really awesome if we could retry the entire test if the afterEach hook fails.

Often people propose some kind of local retry solution like

afterEach(function() {
  tryFiveTimes(function() {
      /* check the console and throw if errors are present */
  });
});

but in this case it won't work, becasue by the time you enter the afterEach hook, the output is already in the console, and every time you check for it you'll get the same result. The only solution is to retry the whole thing (fail the test and run it again with all the hooks).

I understand that before and after hooks are supposed to setup/teardown the environment and if we do retry on all the hook failures it can mess it up, but maybe there can be some kind of explicit API for it. for example

afterEach(function(done) {
    webdriverAPI.getConsoleOutput((output)  => {
        if (output) {
            this.failAndRetry(); // basically does `retries++` and runs the test again if the are still retries left
        } else {
            done();
        }
    });
});

[chipsenkbeil]

Wish there was a non-boilerplate solution to this. Having to add retry logic to my hooks manually is pretty frustrating when I'm dealing with network issues on database cleanup.

[ScottFreeCode]

Regarding the first example of retrying or failing the test based on something that should apply to all of them, you can do it today by using regular functions instead of hooks. Instead of (adapted from the original example with the comments stripped out for simplicity of comparison and the missing promise error handling added):

describe('a button', function() {
    it('shows a modal window when button is clicked', function(done) {
        webdriverAPI.click('.button')
        .then(() => webdriverAPI.isPresentOnThePage('.modal-window'))
        .then(() => done(), done); // NB: second done is to report any errors thrown inside the promise callbacks
    });

    afterEach(function(done) {
        webdriverAPI.getConsoleOutput().then((output) => {
            if (output) {
                return done(new Error(`unexpected output in the console: ${output}`));
            } else {
                done();
            }
        });
    });
});

...you could just convert it so the test calls the error-checking code:

describe('a button', function() {
    it('shows a modal window when button is clicked', function(done) {
        webdriverAPI.click('.button')
        .then(() => webdriverAPI.isPresentOnThePage('.modal-window'))
        .then(validateTestConsole.bind(null, done), done);
    });

    function validateTestConsole(done) {
        webdriverAPI.getConsoleOutput().then((output) => {
            if (output) {
                done(new Error(`unexpected output in the console: ${output}`));
            } else {
                done();
            }
        });
    }
});

(Tangentially, this could be simplified a little by leveraging Mocha's promise support:)

describe('a button', function() {
    it('shows a modal window when button is clicked', function() {
        return webdriverAPI.click('.button')
        .then(() => webdriverAPI.isPresentOnThePage('.modal-window'))
        .then(validateTestConsole);
    });

    function validateTestConsole() {
        return webdriverAPI.getConsoleOutput().then((output) => {
            if (output) {
                throw new Error(`unexpected output in the console: ${output}`);
            }
        });
    }
});

I would argue that this is not only doable without special hook behaviors/functions, but more correct to begin with. The fundamental issue here isn't the retrying. The issue is that you want this check for browser errors to effectively be part of the test: the test fails if there were errors in the browser. As such, it's really test code and belongs in the test; hooks are designed for running cleanup whether the test fails or not, and they aren't designed to be an extension of the tests themselves. The fact that hooks can run for multiple different tests is, I think, incidental when any function can be run for every test (albeit by calling it, but that's not complicated).

---

Retrying cleanup actions within a hook, without retrying the test itself, would be another matter -- I'd like some way to abstract it out that would be useful not only in test code but for anything where a certain amount of error needs to be ignored...

[JoshuaKGoldberg]

This has been pending since 2016. Its total interaction count including 👍s has been roughly one a year. Per #5027, we're trying not to make big changes such as new API additions that aren't highly beneficial. I'm going to go ahead and close this out.

If someone really wants a retry API, please do yell at me. We can re-open this. But please also provide a concrete proposal for what you'd like that API to look like. 🙂