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

Change build state logic #198

Merged
merged 1 commit into from Jan 25, 2015
Merged

Change build state logic #198

merged 1 commit into from Jan 25, 2015

Conversation

ghost
Copy link

@ghost ghost commented Jan 25, 2015

I found some cases where the classes were marked as UNTESTED while they should be marked as EMPTY (classes without implementation code, for example).

I also found some cases where test covered classes that were marked as EMPTY because the executable lines was equal to 0 (not sure if it is a PHP_CodeCoverage issue or a phpDox issue). At least the change fixed both issues.

I created a repository with example code that shows best what the change does in action.

It still follows the Arne Blankerts definition but was modified because
I found some issues like interfaces and empty clases being marked as
untested when they should be marked as empty.

Additionally I found some cases where test covered classes were marked
as empty because the executable lines was equal to 0 (not sure if it is
a PHP_CodeCoverage issue or a phpDox issue).

The change in the XSLT logic solves the issues above. I basically
inverted the logic statements. If there is coverage or executable lines
for the class, it will classify it between untested, failed or passed.
Otherwise, it will be classified as empty.

See: #147 (comment)
theseer added a commit that referenced this pull request Jan 25, 2015
@theseer theseer merged commit 3adda81 into theseer:master Jan 25, 2015
@theseer
Copy link
Owner

theseer commented Jan 25, 2015

I now have some cases that are marked as "empty" despite they do contain code.
For instance when runnig phpDox against itself, I get the class
TheSeer\phpDox\BackendBootstrapApi marked as empty even though it should be untested. Looking at the Coverage XML data it seems that this class is missing from its report. I'll have to check why that is but it also implies we need to handle that case better than claiming the class is "empty".

@ghost
Copy link
Author

ghost commented Jan 26, 2015

It is trickier than it seems. I have a case where a class have code coverage and executable lines but in phpDox's created PHPUnit DOM it is added as a node with the property executable equal to 0.

I really did not investigate the problem for too long as I was only trying to make it work. But if you need help or feedback, just ping me.

@theseer
Copy link
Owner

theseer commented Jan 26, 2015

What's the originating coverage information from PHPUnit's XML say in that regard?

@ghost
Copy link
Author

ghost commented Jan 26, 2015

This is the coverage data:

<!-- coverage/xml/index.xml -->
<file name="SummaryFactory.php" href="Helper/SummaryFactory.php.xml">
  <totals>
    <lines total="72" comments="13" code="59" executable="25" executed="25" percent="100.00%"/>
    <methods count="3" tested="3" percent="100.00%"/>
    <functions count="0" tested="0" percent=""/>
    <classes count="1" tested="1" percent="100.00%"/>
    <traits count="0" tested="0" percent=""/>
  </totals>
</file>

<!-- coverage/xml/Helper/SummaryFactory.php.xml -->
<phpunit xmlns="http://schema.phpunit.de/coverage/1.0">
  <file name="SummaryFactory.php">
    <totals>
      <lines total="72" comments="13" code="59" executable="25" executed="25" percent="100.00%"/>
      <methods count="3" tested="3" percent="100.00%"/>
      <functions count="0" tested="0" percent=""/>
      <classes count="1" tested="1" percent="100.00%"/>
      <traits count="0" tested="0" percent=""/>
    </totals>
    <class name="SummaryFactory" start="7" executable="25" executed="25" crap="7">
      <package full="" name="" sub="" category=""/>
      <namespace name="Api\Helper"/>
      <method name="createSummary" signature="createSummary($results)" start="18" end="29" crap="1" executable="7" executed="7" coverage="100"/>
      <method name="parseBadges" signature="parse($results)" start="34" end="52" crap="3" executable="11" executed="11" coverage="100"/>
      <method name="anonymous function" signature="anonymous function ( $result )" start="61" end="63" crap="1" executable="2" executed="2" coverage="100"/>
      <method name="parseDistances" signature="parseDistances($results)" start="59" end="71" crap="2" executable="0" executed="0" coverage="100"/>
    </class>
    <coverage>
      <line nr="20">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="21">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="23">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="24">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="25">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="26">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="28">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="36">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="38">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="39">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="40">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="41">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="42">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="44">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="48">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="50">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="51">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="52">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="62">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="63">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="65">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
      <line nr="66">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="67">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="68">
        <covered by="SummaryFactoryTest::testGetSummary"/>
      </line>
      <line nr="70">
        <covered by="SummaryFactoryTest::testGetSummary"/>
        <covered by="SummaryFactoryTest::testGetSummaryWhenEmptyResult"/>
      </line>
    </coverage>
  </file>
</phpunit>

And this is the data generated by phpDox:

<class name="SummaryFactory" src="Helper/SummaryFactory.php" xml="classes/Api_Helper_SummaryFactory.xml">
    <enrichments>
        <enrichment type="phpunit">
            <result xmlns="http://schema.phpunit.de/coverage/1.0" error="0" failure="0" incomplete="0" passed="6" risky="0" skipped="0">
            </result>
            <coverage xmlns="http://schema.phpunit.de/coverage/1.0" coverage="100" crap="2" executable="0" executed="0">
            </coverage>
        </enrichment>
    </enrichments>
</class>

@theseer
Copy link
Owner

theseer commented Jan 26, 2015

Okay, that looks like a bug in phpDox as the Coverage Report clearly shows coverage data as well as executable lines.

I'll investigate that.

@theseer theseer self-assigned this Jan 26, 2015
@theseer
Copy link
Owner

theseer commented Feb 6, 2015

I fail to reproduce this. Can you provide me with a stripped down testcase?

@ghost
Copy link
Author

ghost commented Feb 15, 2015

What would be more useful? PHP code?

I tried to reproduce using the similar behavior I found in proprietary production code. I'll look if I'm able to reproduce the code structure more closely.

@theseer
Copy link
Owner

theseer commented Feb 17, 2015

Yeah, having a piece of code I could a) run the test against to get the coverage data and b) have phpdox process would be perfect. I added a tests/data folder with various samples on how I do that already for other tickets - in case you want to take a look.

@theseer
Copy link
Owner

theseer commented Mar 5, 2015

Any updates?

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