Skip to content

Fix tests count when tests were skipped in BeforeClass hook#6490

Merged
sebastianbergmann merged 2 commits intosebastianbergmann:11.5from
nikophil:fix/skipped-in-setup-before-class-count
Feb 4, 2026
Merged

Fix tests count when tests were skipped in BeforeClass hook#6490
sebastianbergmann merged 2 commits intosebastianbergmann:11.5from
nikophil:fix/skipped-in-setup-before-class-count

Conversation

@nikophil
Copy link
Copy Markdown
Contributor

@nikophil nikophil commented Feb 4, 2026

Hello,

when some tests are skipped in a "before class" hook, there is a problem in the output.

Let's say our test suite have:

  • 100 tests in a class which is skipped using #[RequiresPhp('^9.0')]
  • 100 other tests wich will run normally

here what its output looks like:

...............................................................  63 / 200 ( 31%)
.....................................SSSSSSSSSSSSSSSSSSSSSSSSSS 126 / 200 ( 63%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS 189 / 200 ( 94%)
SSSSSSSSSSS                                                     200 / 200 (100%)
 
Time: 00:00.025, Memory: 10.00 MB
 
OK, but some tests were skipped!
Tests: 200, Assertions: 100, Skipped: 100.

(BTW, the output is the same if the tests are skipped by calling self::markTestSkipped() in a "before" hook)

and now, the very same suite, but self::markTestSkipped() is called in a "before class" hook:

...............................................................  63 / 200 ( 31%)
.....................................
 
Time: 00:00.016, Memory: 10.00 MB
 
OK, but some tests were skipped!
Tests: 100, Assertions: 100, Skipped: 1.

We can see several problems in the output, comparing to the previous one:

  • the second line of the progress bar does not display any count
  • skipped tests are not displayed in the progress bar
  • there are only 100 dots int the progress whereas the total displayed at the end of the line is 200
  • the summary seems to not reflect the total number of tests, nor the number of skipped tests

Here is a proposal to make this case coherent with any other mean to skip a full class of tests.

Comment on lines +237 to +239
if ($this->prepared) {
return;
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this condition is needed, but I've mimicked what's done in testSkipped()

Copy link
Copy Markdown
Contributor Author

@nikophil nikophil Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codecov yells about it, and I don't know how to mave this condition true, so I'll remove the condition

@sebastianbergmann sebastianbergmann added feature/test-runner CLI test runner version/11 Something affects PHPUnit 11 version/12 Something affects PHPUnit 12 version/13 Something affects PHPUnit 13 labels Feb 4, 2026
@sebastianbergmann sebastianbergmann self-assigned this Feb 4, 2026
@nikophil nikophil force-pushed the fix/skipped-in-setup-before-class-count branch from 2093629 to 1bc5d60 Compare February 4, 2026 13:47
@sebastianbergmann sebastianbergmann added the type/bug Something is broken label Feb 4, 2026
Comment on lines +17 to +19
SS 2 / 2 (100%)

Time: 00:00, Memory: 10.00 MB
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the impact on this case tolerable?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand what impact you are refering to here. However: in the above, 00:00 and 10.00 MB must be replaced with %s.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I've already fixed this.

I was just underlying that in this case (only one test class which is skipped), the output has changed

@nikophil nikophil force-pushed the fix/skipped-in-setup-before-class-count branch from 1bc5d60 to 9fac5ac Compare February 4, 2026 13:51
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.38%. Comparing base (debf1b7) to head (fea3962).
⚠️ Report is 2 commits behind head on 11.5.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               11.5    #6490   +/-   ##
=========================================
  Coverage     95.37%   95.38%           
- Complexity     7151     7154    +3     
=========================================
  Files           737      738    +1     
  Lines         21853    21864   +11     
=========================================
+ Hits          20843    20854   +11     
  Misses         1010     1010           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nikophil nikophil force-pushed the fix/skipped-in-setup-before-class-count branch from 9fac5ac to fea3962 Compare February 4, 2026 14:06
@sebastianbergmann sebastianbergmann merged commit 5a6ae72 into sebastianbergmann:11.5 Feb 4, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature/test-runner CLI test runner type/bug Something is broken version/11 Something affects PHPUnit 11 version/12 Something affects PHPUnit 12 version/13 Something affects PHPUnit 13

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants