Retry logic is not working when beforeEach() or afterEach() functions fail

[grvk]

Hello,
I've been trying to run a simple suite:

describe("Suite", function() {

    this.retries(1);
    var toFail = true;

    beforeEach(function(){
        console.log("beforeEach");
    });

    afterEach(function() {
        console.log("afterEach");
        if (toFail) {
            toFail = false;
            throw new Error("Cause retry");
        }
    });

    it("Test", function() {
        console.log("it");
    });
});

My expectation was that if it(), beforeEach() or afterEach() fail, then beforeEach() > it () > afterEach() functions are to be executed again. However, it appears that retry logic is not triggered if beforeEach() or afterEach() fail. It works only if the failing function is it()

[longlho]

@grvk some context on why it behaves as it does: #1989 (comment)

[boneskull]

re this (from #1989):

No, just tests, and I'd personally be opposed to the idea of retries for hooks. Hooks serve as setup/teardowns for tests, and can thus be used to cleanup before/after a failed test. For failed hooks (afterEach, after), you can't be sure of the state when retrying. If you have brittle logic in hooks, I'd suggest making a helper function and moving it to be invoked from the specs.

I'm not sure what side of the fence I'm on here. @danielstjules 's point makes sense. Yet, his suggestion of moving the functionality into the specs themselves would create boilerplate, which should be avoided.

@grvk @DmitriiAbramov Why can't you explicitly handle potential errors in your hooks?

[boneskull]

A pattern that may be helpful is this:

function tryToGetCriticalValue() {
  // set a max # of retries here; something like lodash's _.before()
  return getIt()
    .catch(() => tryToGetCriticalValue());
}

describe('something wonky', () => {
  let value;
  beforeEach(() => {
    return tryToGetCriticalValue()
      .then(v => value = v);
  });

  it('should do something with the value', () => {
    // ...
  });
});

[aaronabramov]

@boneskull yeah... if possible, i always try to do local retries. e.g. in e2e tests

beforeEach(retryFiveTimes(() => navigateTo('http://localhost:3000')));

so that it stays in beforeEach and doesn't go outside its scope, but in the case of checking console errors it gets complicated.

it('opens the page', function() {
  clickSomeLink(); // at this point something throw an error (network call to profile_pic.jpg failed)
  validateSomethingIsThere(); // all good, test passes
});

afterEach(function() {
  let output = getConsoleErrorOutput(); // here we get our network error log

  if (output) {
    // even if we retry, it's not going to change anything. the output has been already printed 
    // to the console during `it` block execution
    throw new Error(`some weird stuff got printed in the console: ${output}`);
  }
});

[boneskull]

@DmitriiAbramov The afterEach() above is troubling. Can you create a new issue and describe your use case, and how we can better support it? I'm unsure how you're running this test or why you have to manually retrieve the console output.

That being said, I'm going to close this for now, unless someone can make a reasoned argument against @danielstjules's comment:

Hooks serve as setup/teardowns for tests, and can thus be used to cleanup before/after a failed test. For failed hooks (afterEach, after), you can't be sure of the state when retrying.

@danielstjules It'd be helpful if you can explain exactly why we cannot be sure of the program state?

[aaronabramov]

@boneskull created a new issue with the explanation of the console stuff #2127