Skip to content

Conversation

GrahamCampbell
Copy link
Contributor

No description provided.

phpunit.xml.dist Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I prefer that strict be enabled because it helps greatly in doing accurate code coverage reports; is there a reason that you disabled 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.

Yeh. Strict has a different meaning in phpunit 4. It was causing all the tests to be marked a risky.

This is what happened when I tested it before I sent this pull: https://travis-ci.org/GrahamCampbell/GraphViz/jobs/33164111.

Copy link
Member

Choose a reason for hiding this comment

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

I need to know for sure that when strict is disabled that only code that is covered using an @covers tag is actually shown as being covered. I always understood that was one of the effects of strict. If that proves to be wrong then I'd happily merge it but otherwise this can be tricky.

I don't have the time this evening to test that so that either has to wait until saturday or if you could assert that then it would be awesome!

Copy link
Member

Choose a reason for hiding this comment

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

PHPUnit4 made strict be even more strict :-D That "risky" classification now means that any code that was executed by a test but was not listed by @covers will result in the test being marked as "risky", which is nearly equivalent to a failure. I guess it's supposed to force you into making your test cases even tighter with regard to the dependencies they touch.

Copy link
Member

Choose a reason for hiding this comment

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

That can be resolved by setting the "checkForUnintentionallyCoveredCode" setting to false in the phpUnit config file: http://phpunit.de/manual/4.1/en/strict-mode.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to do that then?

Copy link
Member

Choose a reason for hiding this comment

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

If you could test if that solves the issue that would be absolutely great!

@mvriel
Copy link
Member

mvriel commented Aug 21, 2014

Aside from strict being disabled these are great changes, thank you. I do however use strict for accurate coverage reports (using @covers) and would like to know the intended benefit of disabling it

@GrahamCampbell
Copy link
Contributor Author

Ping @mvriel. I've added back in the safe strict stuff.

@GrahamCampbell
Copy link
Contributor Author

Ping @mvriel.

@mvriel
Copy link
Member

mvriel commented Nov 22, 2014

Hi @GrahamCampbell, I am not sure what went wrong there .. Sorry about not responding sooner. This is perfect and merged. Thanks for pinging me again

mvriel added a commit that referenced this pull request Nov 22, 2014
Composer And Travis Updates
@mvriel mvriel merged commit 767cf6f into phpDocumentor:master Nov 22, 2014
@GrahamCampbell GrahamCampbell deleted the stuff branch November 22, 2014 21:40
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.

3 participants