Skip to content

Add @covers annotations to the tests #272

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

Merged
merged 1 commit into from
Jun 27, 2021

Conversation

oliverklee
Copy link
Collaborator

These make sure that only the lines of the classes the testcases are
about are marked as covered, hence reducing false coverage.

@oliverklee
Copy link
Collaborator Author

Note: This change will reduce the measured code coverage score. On the other hand, the new number will reflect reality much better.

@oliverklee
Copy link
Collaborator Author

Review note: I'll appreciate a critical eye on whether the annotations are complete, or whether we need to add annotations for more covered classes to the test cases.

@sabberworm
Copy link
Collaborator

Review note: I'll appreciate a critical eye on whether the annotations are complete, or whether we need to add annotations for more covered classes to the test cases.

I think the ones you marked as covering \Sabberworm\CSS\Parser are also supposed to cover all of the static parse methods like Color::parse, CSSString::parse, etc.

@oliverklee
Copy link
Collaborator Author

@sabberworm Thanks! I will update the annotations accordingly (and rebase).

These make sure that only the lines of the classes the testcases are
about are marked as covered, hence reducing false coverage.
@oliverklee
Copy link
Collaborator Author

Rebased, updated according to the review comments, and repushed.

@sabberworm
Copy link
Collaborator

Rebased, updated according to the review comments, and repushed.

Thanks

@sabberworm sabberworm merged commit 40e9e89 into MyIntervals:master Jun 27, 2021
@oliverklee oliverklee deleted the task/covers branch June 27, 2021 13:26
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