SpacePossum commented 6 years ago

Sadly, after the release of v2 not all are happy with it. I'm sorry and hereby apologize to everyone who is effected by the changes.

The intent for v2 was to provide an extendable diff library together with some issue resolving which required BC breaks. However some changes were not up to par and need work.

This PR is the start of resolving the reported issue. First of I want to create a nice and clean test suite for the package such that new changes can be reviewed more easy.

The changes are:

Exception - Move to own namespace. (I'm still puzzled why this worked with PSR4)
UnitTest - Code grooming.
- move test to own namespace
- break up DifferTest to dedicated test per class

To be clear, besides the namespace change of the Exceptions, there are no functional changes to the package in this PR (only to its tests).

Ping @keradus if you've time please do a sanity check :)

sebastianbergmann commented 6 years ago

I do not use PSR-4 (or similar) for my components
Please use classmap-based autoloading
Please do not put exceptions into a separate namespace

codecov-io commented 6 years ago

Codecov Report

Merging #73 into master will decrease coverage by 37.31%. The diff coverage is 100%.

@@              Coverage Diff              @@
##             master      #73       +/-   ##
=============================================
- Coverage     99.74%   62.43%   -37.32%     
  Complexity      150      150               
=============================================
  Files            10       10               
  Lines           397      386       -11     
=============================================
- Hits            396      241      -155     
- Misses            1      145      +144

Impacted Files	Coverage Δ	Complexity Δ
src/Output/DiffOnlyOutputBuilder.php	`0% <ø> (-100%)`	`10 <0> (ø)`
src/Differ.php	`73.94% <100%> (-25.22%)`	`52 <0> (ø)`
src/Output/AbstractChunkOutputBuilder.php	`0% <0%> (-100%)`	`8% <0%> (ø)`
src/Output/UnifiedDiffOutputBuilder.php	`4.05% <0%> (-95.95%)`	`26% <0%> (ø)`
src/Parser.php	`100% <0%> (ø)`	`19% <0%> (ø)`	:arrow_down:
src/Chunk.php	`100% <0%> (ø)`	`7% <0%> (ø)`	:arrow_down:
src/Line.php	`100% <0%> (ø)`	`3% <0%> (ø)`	:arrow_down:
src/Diff.php	`100% <0%> (ø)`	`5% <0%> (ø)`	:arrow_down:

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 52a3981...e125c6b. Read the comment docs.

SpacePossum commented 6 years ago

I do not use PSR-4 (or similar) for my components Please use classmap-based autoloading Please do not put exceptions into a separate namespace

all done (I think)

not sure what is going on with the coverage report, no tests have been removed

sebastianbergmann commented 6 years ago

Please keep test case classes in the same namespace as the class they're testing.

SpacePossum commented 6 years ago

Please keep test case classes in the same namespace as the class they're testing.

updated

sebastianbergmann commented 6 years ago

Thanks for your quick responses.

Is this ready to be merged?

SpacePossum commented 6 years ago

Squashed to get a clean commit for the changes, if the tests are green it should be good. However I'm not sure why the coverage report comes back so poorly.

sebastianbergmann commented 6 years ago

Don't worry about the @covers annotations (the reason for low coverage right now). I'll take care of that.

SpacePossum commented 6 years ago

:+1: thanks :)

sebastianbergmann / diff

Clean up the test suit #73

Codecov Report