Type error triggerd by afterEach hook: Cannot read property 'selector' of undefined

[tzolnai]

I experienced an issue with afterEach() hook. Sometimes when the afterEach() should fail on an assertion it fails, but with an exception coming from cypress code.

Current behavior

Cypress fails with a type error, instead of a normal test failure.

Desired behavior

We should not experience a type error in this case.

Test code to reproduce

describe('Uncaught exception in afterEach() hook.', {retries : 0}, function() {
	beforeEach(function() {
		cy.visit('https://googlechrome.github.io/samples/picture-in-picture/');
	});

	afterEach(function() {
		cy.get('a[href=\'http://www.blender.org\']')
			.should('not.have.text', 'www.blender.org');
	});

	it('Failing test.', function() {
		expect(false).to.be.true;
	});
});

It somehow related to that the actual test case is failing too, without it I can't see the same issue.

I see the following error in the log when I try to run this test in headless mode:

1) Uncaught exception in afterEach() hook.
       Failing test.:
     AssertionError: expected false to be true
      at Context.eval (https://googlechrome.github.io/__cypress/tests?p=integration_tests/desktop/writer/test_spec.js:15:24)                                                                   
                                                                                                                                                                                               
  2) Uncaught exception in afterEach() hook.
       "after each" hook for "Failing tes.":
     TypeError: Cannot read property 'selector' of undefined
                                                                                                                                                                                               
Because this error occurred during a `after each` hook we are skipping the remaining tests in the current suite: `Uncaught excep                                                               tion in after...`                                                                                                                                                                              
      at assert (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:168042:33)                                                                                                  
      at assertPartial (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:168067:12)                                                                                           
      at Proxy.<anonymous> (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:168096:12)                                                                                       
      at Proxy.methodWrapper (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:80820:25)                                                                                      
      at applyChainer (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:154670:32)                                                                                            
      at https://googlechrome.github.io/__cypress/runner/cypress_runner.js:154724:16                                                                                                           
      at arrayReduce (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:25417:21)                                                                                              
      at Function.reduce (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:34441:14)                                                                                          
      at applyChainers (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:154701:24)                                                                                           
      at tryCatcher (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:10325:23)                                                                                               
      at Function.Promise.attempt.Promise.try (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:7599:29)                                                                      
      at Context.shouldFn (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:154730:23)                                                                                        
      at Context.should (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:154748:23)                                                                                          
      at Context.<anonymous> (https://googlechrome.github.io/__cypress/runner/cypress_runner.js:169891:21)                                                                                     
      at https://googlechrome.github.io/__cypress/runner/cypress_runner.js:169315:15  

Versions

cypress version: 5.5.0
browser: chromium 78.0.3904.108
OS: openSUSE Leap 15.0

[jennifer-shehane]

Yeah, this seems like unexpected behavior to me. This looks to be a regression from 5.3.0 that was introduced in this PR: #8612

It seems in this case that the ctx._obj is undefined here https://github.com/sainthkh/cypress/blob/develop/packages/driver/src/cypress/chai_jquery.js#L56 cc @sainthkh

Reproducible example

index.html

<html>
<body>
  <a href="http://www.cypress.io">www.cypress.io</a>
</body>
</html>

spec.js

describe('Uncaught exception in afterEach() hook.', () => {
  afterEach(() => {
    cy.wrap('a[href=\'http://www.cypress.io\']')
      .should('not.have.text', 'www.cypress.io')
  })

  it('Failing test.', () => {
    cy.visit('index.html').then(() => {
      expect(false).to.be.true
    })
  })
})

5.2.0

5.3.0

[sainthkh]

I could reproduce the bug. But it made me think that it's not related with #8612.

Because this issue has 2 problems:

1. The assertion in afterEach doesn't fail.
2. assertion is logged 4 times.

But before trying to fix this issue, I want to ask this question. When should we add assertions inside afterEach block? It feels like anti-pattern to me. Simply moving assertion inside the it function works.

[jennifer-shehane]

It seems reasonable to add assertions to afterEach blocks if they'd like.

I tracked down this issue to #8612 - the issue does not occur in the commit prior to that PR.

The assertion has always logged 4 times (not introduced in the PR), but yeah I think that is not what should be happening.

The bug is that the failed assertion in the test body doesn't fail the test - and log the reason the test failed as expect(false).to.be.true. Instead it moves on to the afterEach hook failure and logs that as the reason the test failed.

[sainthkh]

Moving on to the afterEach is natural because the purpose of afterEach is cleaning up the test (like freeing resources, resetting config, etc.). Without this, the next tests would fail.

That's why I asked if it is not an anti-pattern. Asserting is the purpose of it functions, not the afterEach functions.

And I learned that it was called 4 times in afterEach because Cypress was trying to retry the failed test (it's a natural Cypress behavior) and the reporter logged the each try as successful (It's not a natural Cypress behavior). When you change the assertion inside the afterEach to be truthy, the test works correctly.

And just ignoring the failure inside afterEach is also dangerous.

As I said above, the job of afterEach is cleaning up. If an assertion fails inside it, the cleaning up fails and the cleaning-up processes next to it might be skipped because an error is thrown. Then, we cannot reliably test the next tests. That's why the testing frameworks skip the remaining tests. I believe just ignoring failed assertion messages in afterEach and only showing failed test messages are wrong.

[tzolnai]

About having assertions in the afterEach hook:
In my case, we are using afterEach to do cleaning-up after every tests, but we also check whether cleaning-up was successful or not. For this, we use simple cypress assertions. We have an admin panel, where we can check the used resources and we use this panel to make sure every resource is released. The cleaning-up processes are also part of the software, that we are testing with cypress.

[sainthkh]

@tzolnai Then, I think we need to show afterEach error message rather than the failed test, right?

[tzolnai]

@sainthkh: I think this conversation went off a bit. This issue is about the type error and not about whether afterEach hook is correctly called after a test failure or not. @jennifer-shehane pointed out which code and which PR introduced this type error, so I think this is clear, where it needs to be fixed.

[sainthkh]

Currently, should commands inside afterEach don't work. (Actually, it never worked correctly even before this type error. It threw errors but they were ignored. If you want to test freeing resources, I recommend writing new tests about that in it functions, not in afterEach.) Removing them would be the workaround for now.

I'll submit a PR that bypasses this type error.

It seems that we should treat bypassing the type error and making should work correctly inside afterEach as separate issues.

[sainthkh]

As discussed in #9256, #9207 partially solved this problem by throwing error when undefined is given as a subject like below:

But it doesn't bring back the before-5.3.0 behavior. To solve this problem, we need to do 2 things:

1. Show multiple errors if there's a failure in after or afterEach. -> It's the goal of feat: support multiple errors in each test #8277.
2. Don't show failed attempts as success on afterEach.

[cypress-app-bot]

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

[cypress-app-bot]

This issue has been closed due to inactivity.