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

PCOV #663

Closed
wants to merge 6 commits into from
Closed

PCOV #663

wants to merge 6 commits into from

Conversation

krakjoe
Copy link
Contributor

@krakjoe krakjoe commented Jan 25, 2019

This is a clean PR for PCOV, with commits separated sensibly.

@krakjoe krakjoe changed the title Pcov PCOV Jan 25, 2019
@krakjoe
Copy link
Contributor Author

krakjoe commented Jan 25, 2019

@sebastianbergmann anything else needed from me ?

@sebastianbergmann
Copy link
Owner

Not that I can think of right now. Just need time to research, review, and decide.

composer.json Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
- composer self-update
- composer clear-cache
- export COMPOSER_ROOT_VERSION=6.1.99

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is still needed, because we still have a circular dep, phpunit depends php-code-coverage

src/Driver/PCOV.php Outdated Show resolved Hide resolved

if ($collect) {
\pcov\clear();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the indentation is a bit off here, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see it ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff --git a/src/CodeCoverage.php b/src/CodeCoverage.php
index 69b4bc0..62e28b4 100644
--- a/src/CodeCoverage.php
+++ b/src/CodeCoverage.php
@@ -13,8 +13,8 @@
 use PHPUnit\Runner\PhptTestCase;
 use PHPUnit\Util\Test;
 use SebastianBergmann\CodeCoverage\Driver\Driver;
-use SebastianBergmann\CodeCoverage\Driver\PHPDBG;
 use SebastianBergmann\CodeCoverage\Driver\PCOV;
+use SebastianBergmann\CodeCoverage\Driver\PHPDBG;
 use SebastianBergmann\CodeCoverage\Driver\Xdebug;
 use SebastianBergmann\CodeCoverage\Node\Builder;
 use SebastianBergmann\CodeCoverage\Node\Directory;
@@ -906,9 +906,9 @@ private function selectDriver(Filter $filter): Driver
             return new Xdebug($filter);
         }

-       if ($runtime->hasPCOV()) {
+        if ($runtime->hasPCOV()) {
             return new PCOV($filter);
-       }
+        }

         throw new RuntimeException('No code coverage driver available');
     }
diff --git a/src/Driver/PCOV.php b/src/Driver/PCOV.php
index 941650e..12f7643 100644
--- a/src/Driver/PCOV.php
+++ b/src/Driver/PCOV.php
@@ -9,7 +9,6 @@
  */
 namespace SebastianBergmann\CodeCoverage\Driver;

-use SebastianBergmann\CodeCoverage\RuntimeException;
 use SebastianBergmann\CodeCoverage\Filter;

 /**
@@ -21,7 +20,6 @@ final class PCOV implements Driver
 {
     public function __construct(Filter $filter = null)
     {
-
     }

     /**
@@ -37,19 +35,19 @@ public function start(bool $determineUnusedAndDead = true): void
      */
     public function stop(): array
     {
-       \pcov\stop();
+        \pcov\stop();

-       $waiting = \pcov\waiting();
-       $collect  = [];
+        $waiting  = \pcov\waiting();
+        $collect  = [];

-       if ($waiting) {
-               $collect = \pcov\collect(\pcov\inclusive, $waiting);
+        if ($waiting) {
+            $collect = \pcov\collect(\pcov\inclusive, $waiting);

-               if ($collect) {
-                       \pcov\clear();
-               }
-       }
+            if ($collect) {
+                \pcov\clear();
+            }
+        }

-       return $collect;
+        return $collect;
     }
 }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, sorry - I ran

$ composer global require friendsofphp/php-cs-fixer:^2.14.0

followed by

$ php-cs-fixer fix --diff --verbose

There were some unrelated fixes applied to a test case, which I have removed from the patch above.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is off for me too. I think it's caused by the mix of tabs and spaces. Same in CodeCoverage.php.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already fixed that locally in a branch where I am preparing this before merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks people, but I'm going to just let Sebastian fix this upon merge, since he has to make minor edits anyway, and I don't want to make any more noise.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. Would make things easier for me if this PR would not be altered anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@codecov
Copy link

codecov bot commented Jan 28, 2019

Codecov Report

Merging #663 into master will increase coverage by 0.99%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #663      +/-   ##
============================================
+ Coverage     85.16%   86.15%   +0.99%     
- Complexity      805      806       +1     
============================================
  Files            32       32              
  Lines          2534     2536       +2     
============================================
+ Hits           2158     2185      +27     
+ Misses          376      351      -25
Impacted Files Coverage Δ Complexity Δ
src/CodeCoverage.php 75.25% <100%> (+1.4%) 181 <0> (+1) ⬆️
src/Report/Xml/Facade.php 92.3% <0%> (+0.76%) 33% <0%> (ø) ⬇️
src/Report/Html/Renderer/Directory.php 86.04% <0%> (+2.32%) 6% <0%> (ø) ⬇️
src/Node/File.php 78.6% <0%> (+3.29%) 88% <0%> (ø) ⬇️
src/Node/Directory.php 95.86% <0%> (+4.95%) 51% <0%> (ø) ⬇️
src/Filter.php 95.65% <0%> (+8.69%) 25% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afe4b8a...75ee8b6. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jan 28, 2019

Codecov Report

Merging #663 into master will increase coverage by 0.99%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #663      +/-   ##
============================================
+ Coverage     85.16%   86.15%   +0.99%     
- Complexity      805      806       +1     
============================================
  Files            32       32              
  Lines          2534     2536       +2     
============================================
+ Hits           2158     2185      +27     
+ Misses          376      351      -25
Impacted Files Coverage Δ Complexity Δ
src/CodeCoverage.php 75.25% <100%> (+1.4%) 181 <0> (+1) ⬆️
src/Report/Xml/Facade.php 92.3% <0%> (+0.76%) 33% <0%> (ø) ⬇️
src/Report/Html/Renderer/Directory.php 86.04% <0%> (+2.32%) 6% <0%> (ø) ⬇️
src/Node/File.php 78.6% <0%> (+3.29%) 88% <0%> (ø) ⬇️
src/Node/Directory.php 95.86% <0%> (+4.95%) 51% <0%> (ø) ⬇️
src/Filter.php 95.65% <0%> (+8.69%) 25% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afe4b8a...75ee8b6. Read the comment docs.

@sebastianbergmann
Copy link
Owner

Looks like Travis is happy now. I'll try to find time ASAP to play with PCOV. Thanks for putting all that effort in!

@sebastianbergmann
Copy link
Owner

Merged manually. Will be released alongside PHPUnit 8 on Friday.

@shulard
Copy link

shulard commented Feb 6, 2019

Hello,

Is there a way to test the PCOV driver with PHPUnit 7 or will it be a PHPUnit 8+ only feature ?

Thanks

@sebastianbergmann
Copy link
Owner

You need PHPUnit 8 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants