Skip to content

QA: use the most specific PHPUnit assertion possible [2]#467

Open
jrfnl wants to merge 1 commit intotrunkfrom
JRF/tests/improve-assertions-used-2
Open

QA: use the most specific PHPUnit assertion possible [2]#467
jrfnl wants to merge 1 commit intotrunkfrom
JRF/tests/improve-assertions-used-2

Conversation

@jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Feb 22, 2026

Context

  • Improve tests

Summary

This PR can be summarized in the following changelog entry:

  • Improve tests

Relevant technical choices:

This is a long established best practice.

PHPUnit contains a variety of assertions and the ones available have been extended hugely over the years.

To have the most reliable tests, the most specific assertion should be used.

Most notably, this changes calls to assertEmpty() to different assertions as assertEmpty() is a completely type-loose assertion.

Refs:

@enricobattocchi Tagging you as reviewer to verify that the updated assertions are documenting the intended behaviour of the application (and not accidental behaviour). Feel free to change/fix any assertions you deem incorrect.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • N/A

This is a long established best practice.

PHPUnit contains a variety of assertions and the ones available have been extended hugely over the years.

To have the most reliable tests, the most specific assertion should be used.

Most notably, this changes calls to `assertEmpty()` to different assertions as `assertEmpty()` is a completely type-loose assertion.

Refs:
* https://phpunit.de/manual/9.6/en/appendixes.assertions.html#appendixes.assertions.assertEmpty
@coveralls
Copy link

Pull Request Test Coverage Report for Build 22267807042

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 59.933%

Totals Coverage Status
Change from base Build 22267764076: 0.0%
Covered Lines: 1614
Relevant Lines: 2693

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants