Skip to content

Clean up the test suit#73

Merged
sebastianbergmann merged 1 commit intosebastianbergmann:masterfrom
SpacePossum:master_fixes
Dec 14, 2017
Merged

Clean up the test suit#73
sebastianbergmann merged 1 commit intosebastianbergmann:masterfrom
SpacePossum:master_fixes

Conversation

@SpacePossum
Copy link
Copy Markdown
Contributor

@SpacePossum SpacePossum commented Dec 14, 2017

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
Copy link
Copy Markdown
Owner

sebastianbergmann commented Dec 14, 2017

  • 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
Copy link
Copy Markdown

codecov-io commented Dec 14, 2017 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@              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%> (ø) ⬇️
src/Chunk.php 100% <0%> (ø) 7% <0%> (ø) ⬇️
src/Line.php 100% <0%> (ø) 3% <0%> (ø) ⬇️
src/Diff.php 100% <0%> (ø) 5% <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 52a3981...e125c6b. Read the comment docs.

@SpacePossum
Copy link
Copy Markdown
Contributor Author

SpacePossum commented Dec 14, 2017 •
edited
Loading

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

@SpacePossum SpacePossum changed the title Output format issues Clean up the test suit Dec 14, 2017
@SpacePossum SpacePossum mentioned this pull request Dec 14, 2017
Merged
@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Dec 14, 2017

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

@SpacePossum
Copy link
Copy Markdown
Contributor Author

SpacePossum commented Dec 14, 2017

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

updated

@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Dec 14, 2017

Thanks for your quick responses.

Is this ready to be merged?

@SpacePossum
Copy link
Copy Markdown
Contributor Author

SpacePossum commented Dec 14, 2017

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 sebastianbergmann merged commit c6f30b2 into sebastianbergmann:master Dec 14, 2017
@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Dec 14, 2017

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

@SpacePossum
Copy link
Copy Markdown
Contributor Author

SpacePossum commented Dec 14, 2017

👍 thanks :)

@SpacePossum SpacePossum deleted the master_fixes branch December 14, 2017 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SpacePossum @sebastianbergmann @codecov-io