[TASK] Do not assertX but fail() in tearDown() #660
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
phpunit has a best practice that tests should usually have at least one assertion to be sure they actually do something. All assertions thus raise a counter that is checked after test execution. If zero, phpunit marks the test risky with "test has no assertion". There are two ways to suppress this: * Setting beStrictAboutTestsThatDoNotTestAnything="false" via phpunit config * Adding #[DoesNotPerformAssertions] attribute to single tests to actively mark tests that do not assert something as legit Our abstract UnitTestCase spoils this by always doing assertions in tearDown(). The patch turns these assertions into check+fail() code instead. Unit tests that don't have assertions for whatever reason are now properly marked as risky as intended by phpunit. Releases: main
sbuerk
approved these changes
Nov 30, 2024
reviewtypo3org
pushed a commit
to TYPO3/typo3
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer req --dev typo3/testing-framework:^9.1.0 [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87293 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
reviewtypo3org
pushed a commit
to TYPO3/typo3
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer u typo3/testing-framework [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87289 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: core-ci <typo3@b13.com>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/backend
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer req --dev typo3/testing-framework:^9.1.0 [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87293 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/core
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer req --dev typo3/testing-framework:^9.1.0 [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87293 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/extbase
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer req --dev typo3/testing-framework:^9.1.0 [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87293 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/install
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer req --dev typo3/testing-framework:^9.1.0 [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87293 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/backend
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer u typo3/testing-framework [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87289 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: core-ci <typo3@b13.com>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/core
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer u typo3/testing-framework [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87289 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: core-ci <typo3@b13.com>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/extbase
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer u typo3/testing-framework [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87289 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: core-ci <typo3@b13.com>
TYPO3IncTeam
pushed a commit
to TYPO3-CMS/install
that referenced
this pull request
Nov 30, 2024
New TF does not raise assertions in tearDown() anymore [1] which makes phpunit detect more unit tests that don't assert something. Some affected tests are legit and receive the #[DoesNotPerformAssertions] attribute. Some others were actually broken and are fixed to for instance now expect calls on mocks. Further minor cleanups along the way. > composer u typo3/testing-framework [1] TYPO3/testing-framework#660 Resolves: #105730 Releases: main, 13.4 Change-Id: I75289b8242749cb596214566254e7a3da1f7e551 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87289 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: core-ci <typo3@b13.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
phpunit has a best practice that tests should usually have at least one assertion to be sure they actually do something. All assertions thus raise a counter that is checked after test execution. If zero, phpunit marks the test risky with "test has no assertion".
There are two ways to suppress this:
Our abstract UnitTestCase spoils this by always doing assertions in tearDown(). The patch turns these
assertions into check+fail() code instead. Unit tests that don't have assertions for whatever reason are now properly marked as risky as intended by phpunit.
Releases: main