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

expects should fail a test even if it's inside a try/catch. #3917

Closed
sebmarkbage opened this issue Jun 26, 2017 · 14 comments
Closed

expects should fail a test even if it's inside a try/catch. #3917

sebmarkbage opened this issue Jun 26, 2017 · 14 comments

Comments

@sebmarkbage
Copy link

Currently expect helpers throw if there's an error. This lets them be caught in various global error handlers. This makes it difficult to test environment that has expected thrown behavior or explicitly tests custom error handling behavior.

Failing assertions should set a global flag that fails the test even if there's a try/catch involved.

For example, these tests should fail:

  it('should fail this', () => {
    function frameworkCode(callback) {
      try {
        callback();
      } catch (x) {
        console.log('An error was handled by the framework.');
      }
    }
    function callback() {
      expect(false).toBe(true);
    }
    frameworkCode(callback);
  });

  it('should fail this too', () => {
    function callback() {
      expect(false).toBe(true);
      throw new Error();
    }
    expect(callback).toThrow();
  });
@sebmarkbage
Copy link
Author

For real world use cases see React repo that tests lots of behavior that is expected to throw but is also expected to issue console warnings, or not issue console warnings.

@sebmarkbage
Copy link
Author

sebmarkbage commented Jun 26, 2017

The current behavior also cover up error like this one:

  it('should fail this too', () => {
    function returnTrue() {
      return false;
    }
    function callback() {
      expect(returnTrue()).toBe(true);
      if (false) {
        throw new Error();
      }
    }
    expect(callback).toThrow();
  });

There are two errors here, but they cancel out each other so it looks like it passes but it should fail.

@sebmarkbage
Copy link
Author

Another case of bugs being covered up by this facebook/react#10331

@aaronabramov
Copy link
Contributor

we already have a state where keep all the information about our expects for each test.
It should be really easy to implement.

should we make it configureable, or should we always fail the test if any of the expect's throw?

@sebmarkbage
Copy link
Author

It might be nice to make it configurable as an upgrade path but I don't see why you'd expect anything else.

@cpojer
Copy link
Member

cpojer commented Aug 1, 2017

I agree this needs fixing, I don't agree this needs a configuration. I'm fine with the breaking change here, it exposes bugs.

@marxlow
Copy link

marxlow commented Nov 15, 2017

I am facing the same problem with jest that expects are not working as they should inside try-catch blocks.

it('I should be failing', () => {
  try {
    IWillNotThrowAnyError();
    
    // Code should reach here
    expect(false).toEqual(true);
  } catch(error) {
 
   // Code will never reach here
    console.log('Caught an error!');
    expect(true).toEqual(true);
  }
}

The jest test above oddly passes.

@sebmarkbage
Copy link
Author

cc @bvaughn

@cfnelson
Copy link

Experiencing same behaviour on Jest: 22.0.5, whilst using expect within a try/catch block.

Below is a minimal working example of the unexpected behaviour (without). The expect is called as expect.assertions(1); passes without issue, console.error also reports the matcherResult which indicates it should have failed.

test("It should FAIL, it reports PASS and logs error to console.",  () => {
  expect.assertions(1);
  try {
    expect(false).toEqual(true);
  } catch (error) {
    console.error('😭😭😭', error);
  }
});

// Expect reports the below matcherResult
// matcherResult: { 
//  actual: false,
//  expected: true,
//  message: [Function],
//  name: 'toEqual',
//  pass: false } }

Passing test that should fail

Interestingly expect.assertions() behaves differently to expect() and will FAIL the test whilst it's within a try/catch block. See below example & output.

test("It should FAIL, it expects two assertions and it FAILS for receiving only one assertion.",  () => {
  try {
    expect.assertions(2);
    expect(true).toEqual(true);
  } catch (error) {
    console.error('😭😭😭', error);
  }
});

Failing test the should fail

Hopefully this will be helpful to someone. 😄

kincaidoneil added a commit to interledgerjs/ilp-plugin-ethereum that referenced this issue Sep 17, 2018
- due to jestjs/jest#3917, Jest assertions weren't working from within Stream event handlers
- miscellaneous fixes
@cvle
Copy link

cvle commented Mar 8, 2019

We also ran into this. As a workaround we added a separate global function expectAndFail that wraps the original expect but fails the test immediately when the assertion fails.

https://gist.github.com/cvle/e4496b733889c137e9f19dddf861fdd4

You could also replace global.expect with this if you really wanted, but it's a breaking change for some libraries that depends on the non failing behavior.

@esamattis
Copy link

That's clever, thanks for sharing!

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 26, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants