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

Tests: various improvements #308

Merged
merged 5 commits into from
Apr 12, 2022
Merged

Tests: various improvements #308

merged 5 commits into from
Apr 12, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 12, 2022

👉🏻 Based on reviewing the code coverage reports.


Numbers: fix incorrect code coverage reporting

PR #293 split some code from the getCompleteNumber() method off into a separate updateResult() method used only by the getCompleteNumber() method, however code coverage was not being recorded for that new method as no @covers tag had been added for that method to the GetCompleteNumberTest class.

Fixed now.

TokenNameTest: fix incorrect test cases

Turns out those were still PHPCS native tokens, not PHP native ones 😂

BCFile\GetConditionTest::testNonConditionalToken() fix target token

With the changes in PHP 8.0, the T_STRING (namespace name) target token may not exist as it may be tokenized as T_NAME_QUALIFIED instead.

With this change, the correct token should now still be retrieved to allow the test to test what it is supposed to test.

UtilityMethodTestCaseTest: split off the test for the expectPhpcsException() method

The UtilityMethodTestCase::expectPhpcsException() method provides a polyfill for changed PHPUnit functionality. To allow for testing this polyfill properly, it should be tested without the PHPUnit Polyfills active as external standards using the UtilityMethodTestCase may not implement the PHPUnit Polyfills.

To do so, the tests specific to the UtilityMethodTestCase::expectPhpcsException() method have been split off into their own test class which extends the UtilityMethodTestCase directly, instead of extending the PolyfilledTestCase.

GetCompleteNumberTest: add test for int max handling

While the value of PHP_INT_MAX will differ depending on whether PHP is compiled as 32-bits or 64-bits, this test value should convert to float in both cases.

PR 293 split some code from the `getCompleteNumber()` method off into a separate `updateResult()` method used only by the `getCompleteNumber()` method, however code coverage was not being recorded for that new method as no `@covers` tag had been added for that method to the `GetCompleteNumberTest` class.

Fixed now.
Turns out those were still PHPCS native tokens, not PHP native ones 😂
With the changes in PHP 8.0, the `T_STRING` (namespace name) target token _may_ not exist as it _may_ be tokenized as `T_NAME_QUALIFIED` instead.

With this change, the correct token should now still be retrieved to allow the test to test what it is supposed to test.
…eption()` method

The `UtilityMethodTestCase::expectPhpcsException()` method provides a polyfill for changed PHPUnit functionality. To allow for testing this polyfill properly, it should be tested without the PHPUnit Polyfills active as external standards using the `UtilityMethodTestCase` may not implement the PHPUnit Polyfills.

To do so, the tests specific to the `UtilityMethodTestCase::expectPhpcsException()` method have been split off into their own test class which extends the `UtilityMethodTestCase` directly, instead of extending the `PolyfilledTestCase`.
While the value of `PHP_INT_MAX` will differ depending on whether PHP is compiled as 32-bits or 64-bits, this test value should convert to float in both cases.
@jrfnl
Copy link
Member Author

jrfnl commented Apr 12, 2022

There's something seriously off with the Coveralls reporting. I've opened an issue about it.

The actual real new code coverage is 97.3%, which is an increase of +0.6%, so merging.

@jrfnl jrfnl merged commit 816ffcc into develop Apr 12, 2022
@jrfnl jrfnl deleted the feature/tests-various-fixes branch April 12, 2022 14:10
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.

None yet

1 participant