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

Encapsulate raw coverage data from drivers #748

Conversation

dvdoug
Copy link
Contributor

@dvdoug dvdoug commented May 13, 2020

Hi @sebastianbergmann

Please find attached a PR for the first piece of the enabling work for #380. The goal of this particular PR is not to add new functionality, merely to lay groundwork.

What does this PR do?

  • It replaces the return type of array from Driver->stop() with a new object RawCodeCoverageData, with consequential adjustments to the rest of the code
  • It replaces the direct data manipulation that was previously inside CodeCoverage with usage of named methods on RawCodeCoverageData
  • In the Xdebug driver, it adds the ability to accept and propagate driver data in either the traditional line only format, or the extended branch data format

Please note this PR does not adjust the internal $this->data array used by the CodeCoverage class. I do plan on encapsulating that too, but it will be in a different PR.

@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #748 into master will increase coverage by 0.15%.
The diff coverage is 79.68%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #748      +/-   ##
============================================
+ Coverage     83.10%   83.25%   +0.15%     
- Complexity      834      845      +11     
============================================
  Files            35       37       +2     
  Lines          2450     2472      +22     
============================================
+ Hits           2036     2058      +22     
  Misses          414      414              
Impacted Files Coverage Δ Complexity Δ
src/Driver/PHPDBG.php 0.00% <0.00%> (ø) 13.00 <5.00> (ø)
src/Driver/Xdebug.php 0.00% <0.00%> (ø) 6.00 <1.00> (ø)
src/Driver/PCOV.php 33.33% <50.00%> (ø) 3.00 <1.00> (ø)
src/CodeCoverage.php 72.16% <77.77%> (ø) 181.00 <62.00> (ø)
...c/Exception/UnknownCoverageDataFormatException.php 100.00% <100.00%> (ø) 1.00 <1.00> (?)
src/RawCodeCoverageData.php 100.00% <100.00%> (ø) 10.00 <10.00> (?)

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 0be1afd...46d3c0c. Read the comment docs.

@dvdoug dvdoug force-pushed the encapsulate_raw_data_from_drivers branch from 6208dce to b7957eb Compare May 13, 2020 20:52
@dvdoug dvdoug force-pushed the encapsulate_raw_data_from_drivers branch from b7957eb to 46d3c0c Compare May 13, 2020 20:56
@sebastianbergmann sebastianbergmann merged commit 3203df5 into sebastianbergmann:master May 14, 2020
@sebastianbergmann
Copy link
Owner

Thanks! I have made some minor adjustments after merging this.

@dvdoug dvdoug deleted the encapsulate_raw_data_from_drivers branch May 14, 2020 11:25
@dvdoug
Copy link
Contributor Author

dvdoug commented May 14, 2020

👍

*/
private $lineData = [];

public function __construct(array $rawCoverage = [])

Choose a reason for hiding this comment

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

I do not like the fact that the constructor has to guess the format of the array passed in. Can we please make the constructor private and add separate static methods, named constructors, for each supported format? For instance: fromXdebugWithoutPathCoverage(), fromXdebugWithPathCoverage(), etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing

Choose a reason for hiding this comment

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

Thanks!

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.

2 participants