Make WordPress Core

Opened 4 years ago

Closed 3 months ago

#42045 closed defect (bug) (wontfix)

@expectedIncorrectUsage annotation fails under specific scenerio

Reported by: thekt12 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: close
Focuses: Cc:


Suppose if there is test case(the function with @expectedIncorrectUsage annotation above it) with two functions where one of the function doesn't make any call to _doing_it_wrong whereas other function makes more than one call to _doing_it_wrong.

as The expected result is

Tests: 1, Assertions: 2, Failures: 1.

But it returns

OK (1 test, 2 assertions)

The @expectedIncorrectUsage should check if each function has made a call to the _doing_it_wrong instead of checking the number of time _doing_it_wrong was called in a test case and mapping it to the number of assertion

Earlier Reported Case:

Change History (4)

#1 @swissspidy
4 years ago

  • Component changed from General to Build/Test Tools
  • Version trunk deleted

My advice is usually to only write one assertion per test. When you have multiple assertions per test method it's just harder to debug and not very readable. If you'd do that in #42040, you don't run into this problem.

I don't think we should encourage having multiple assertions per test by "fixing" the @expectedIncorrectUsage annotation.

#2 @thekt12
4 years ago

@swissspidy yes one test case per assertion will definitely solve this issue, but i still don't have a way to check if the call to _doing_it_wrong was made from my part of snippet or not.

Also, I feel @expectedIncorrectUsage needs to be fixed as there is a chance that other developer will be able to write failed test cases and still make it look okay. Or if we can make one assetion per test as a testing standard in wordpess and scripts should be written to ensure that. But the later will involve most of the test already present to be broken down and re written to different test functions.

#3 @johnbillion
4 years ago

  • Keywords close added

This isn't really any different from PHPUnit's built-in expectedException functionality. There's no way to assert exactly which part of the code threw the exception. The best way to tackle this is to write small, tightly focused tests to minimise the chance that the unexpected usage is triggered elsewhere.

#4 @hellofromTonya
3 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I agree with both @johnbillion and @swissspidy about:

  • writing single assertions in a single test method
  • not encouraging multiple assertions per test

For multiple scenarios, a data provider is a good choice. Why? Each scenario is run individually in the test method.

As it's been 4 years since the ticket was marked as a close candidate with no follow-up, closing this ticket.

Note: See TracTickets for help on using tickets.