-
Notifications
You must be signed in to change notification settings - Fork 160
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
fix: Missing test coverage #2474
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2474 +/- ##
=============================================
- Coverage 76.55% 64.88% -11.67%
- Complexity 267 2574 +2307
=============================================
Files 138 214 +76
Lines 17457 23044 +5587
Branches 976 0 -976
=============================================
+ Hits 13364 14953 +1589
- Misses 3573 8091 +4518
+ Partials 520 0 -520
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 132 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
BenchmarksBenchmark execution time: 2024-02-29 11:33:48 Comparing candidate commit fdbf9ae in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 182 metrics, 0 unstable metrics. |
@@ -1316,7 +1318,7 @@ test_scenario_%: | |||
$(Q) $(COMPOSER_TESTS) scenario $* | |||
|
|||
merge_coverage_reports: | |||
$(PHPCOV) merge --clover reports/coverage.xml reports/cov | |||
php -d memory_limit=-1 $(PHPCOV) merge --clover reports/coverage.xml reports/cov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason behind memory_limit=-1
here is the number of .cov
files being processed. I'm talking about 100s of coverage files being merged (1 per request + 1 per test suite).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, let's merge it before the branches diverge :-)
6c7bf49
to
69b3df4
Compare
Just did a final rebase onto master to be sure everything is good 👍 Will merge afterwards |
7e4d258
to
3655a4f
Compare
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. If you need to update snapshots, please refer to CONTRIBUTING.md |
Description
Oops, I guess I forgot about them in the initial PR #2464
Two Issues:
test_integrations_*,
but there was still no web test coverage. This is because a newProcess
is spawned in the web tests. Therefore, phpunit+xdebug doesn't have any visibility over what's happening, and no coverage is captured (despite it being where the valuable information lies). To fix this issue, a script (save_code_coverage.php
) will be prepended to the queried file to collect the code coverage manually. These generated code coverage files will be saved inreports/cov
, where they can be merged to a clover file usingmake merge_coverage_reports.
Notes:
tests/Appsec/Mock.php
was modified to prevent its element from being loaded twice. This happened because of the introduction ofsave_code_coverage.php
, which requiresvendor/autoload.php
"Backing up auto_prepend_file '%s'"
is moved to a debug-type one, as it would 1. make the tests fail and 2. shouldn't be a warning.tracer-integrations
andtracer-php-api
flags have been merged into one flag,tracer-php
. This makes it more comprehensive.Reviewer checklist