refactor(agent): improve phpunit instrumentation #725
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
Use file that is loaded also on PHP 8.2 with include/require optimizations that don't execute files without executable statements.
PHPUnit passes skipped tests to `addError` as well as `endTest` regardless of the version. The difference is that in PHPUnit 5.x it is not possible to obtain test outcome for skipped test - `$test.getStatus()` returns 'null'. Because of that, `endTest` instrumentation bails out and does not generate test event and therefore `addError` needs to be instrumented. It is possible to obtain test outcome for skipped tests in PHPUnit 9.x. However, the `$time` for skipped test is not received in `endTest` as a valid double and therefore it needs to be 'fixed'. Moreover testcase.getStatusMessage returns 'null' for tests skipped due to dependency failure and therefore it is not possible to expect it in the test event.
Codecov Report
@@ Coverage Diff @@
## oapi #725 +/- ##
==========================================
+ Coverage 78.88% 79.02% +0.14%
==========================================
Files 191 191
Lines 26749 26757 +8
==========================================
+ Hits 21102 21146 +44
+ Misses 5647 5611 -36
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ZNeumann
approved these changes
Sep 6, 2023
zsistla
reviewed
Sep 6, 2023
agent/php_execute.c
Outdated
| @@ -491,8 +491,8 @@ static nr_library_table_t libraries[] = { | |||
| * /usr/local/bin. While BaseTestRunner isn't the very first file to load, | |||
There was a problem hiding this comment.
Should the comment be updated since we don't use BaseTestRunner anymore?
There was a problem hiding this comment.
Good catch! Comment updated in bd240c9.
zsistla
approved these changes
Sep 6, 2023
Remove a reference to a file no longer used to detect PHPUnit framework.
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.
improve phpunit detection
Use file that is loaded also on PHP 8.2 with include/require optimizations that
don't execute files without executable statements.
improve skipped tests handling
PHPUnit passes skipped tests to
addErroras well asendTestregardless ofthe version. The difference is that in PHPUnit 5.x it is not possible to obtain
test outcome for skipped test -
$test.getStatus()returns 'null'. Because ofthat,
endTestinstrumentation bails out and does not generate test event andtherefore
addErrorneeds to be instrumented. It is possible to obtain testoutcome for skipped tests in PHPUnit 9.x. However, the
$timefor skipped testis not received in
endTestas a valid double and therefore it needs to be'fixed'. Moreover testcase.getStatusMessage returns 'null' for tests skipped due
to dependency failure and therefore it is not possible to expect it in the test
event.