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

Detector suggestion: Expecting Error in unit tests #311

Open
ThrawnCA opened this issue Oct 30, 2018 · 2 comments
Open

Detector suggestion: Expecting Error in unit tests #311

ThrawnCA opened this issue Oct 30, 2018 · 2 comments
Labels

Comments

@ThrawnCA
Copy link
Contributor

Tripped myself up recently when unit-testing some code that is meant to handle a subset of java.lang.Error (specifically, it's a scheduled job that shouldn't completely kill the scheduler if a particular invocation encounters out-of-memory errors).

The thing is, when writing the unit tests that threw a different (not to be handled) Error, I told TestNG to expect the test case to throw Error. Which meant that when I made a mistake in the test, the resulting AssertionError didn't cause a test failure...

Can a new TestNG/JUnit bug be added to detect cases where tests expect AssertionError or a superclass?

Sample:

// oops, this will swallow AssertionError!
@Test(expectedExceptions = Error.class)
public void shouldNotHandleFatalErrors() {
    when(dao.findAll()).thenThrow(new UnknownError("test error"));

    service.new ReportDeliveryTask(Frequency.DAILY);
    // oops, the exception won't be thrown until we actually "run" the task

    fail("Should have aborted due to fatal error");
}

(The fail line was triggering, but the test still passed...mea culpa)

@mebigfatguy
Copy link
Owner

so if you expect Error, then have any kind of Assert.xxx code, report that?

@ThrawnCA
Copy link
Contributor Author

It's more "if you expect AssertionError or any superclass of it, report that." IMO you don't need to look for an actual assertion in the method body. AssertionError is pretty fundamental to the idea of a unit test, so it almost never makes sense to swallow it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants