Skip to content

CoveredTestResultPerTestMethod API#95

Merged
danglotb merged 10 commits intoSTAMP-project:masterfrom
andre15silva:covered-test-result-api
Jun 24, 2021
Merged

CoveredTestResultPerTestMethod API#95
danglotb merged 10 commits intoSTAMP-project:masterfrom
andre15silva:covered-test-result-api

Conversation

@andre15silva
Copy link
Copy Markdown
Contributor

@andre15silva andre15silva commented Jun 23, 2021

This PR introduces a new API in test-runner's EntryPoint.

It's purpose is to return a CoveredTestResultPerTestMethod object, which contains the information relative to the coverage per test method, as well as the test execution results.

@andre15silva
Partially implement CoveredTestResultPerTestMethod API
4a372d4 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Add assertions for coverage map
39d6008 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Rename new runner classes
7caa754 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Fix runner class names in EntryPoint
2e1b482 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Add JUnit4JacocoRunnerCoveredResultPerTestMethodTest
1170660 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Copy link
Copy Markdown
Contributor Author

andre15silva commented Jun 23, 2021 •
edited
Loading

If you want to review the current state of the PR, the content shouldn't change much.

I'm working on a few more test cases to cover everything new, cleaning and documenting the code.

@andre15silva andre15silva mentioned this pull request Jun 23, 2021
Closed
@monperrus
Copy link
Copy Markdown
Member

monperrus commented Jun 24, 2021

Cool, thanks @andre15silva. @danglotb would you be available for a code review?

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 24, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 968000076

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 173 of 307 (56.35%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.1%) to 43.777%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/eu/stamp_project/testrunner/listener/CoveredTestResultPerTestMethod.java 0 1 0.0%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JUnit4JacocoRunnerCoveredResultPerTestMethod.java 18 20 90.0%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JUnit5JacocoRunnerCoveredResultPerTestMethod.java 18 20 90.0%
src/main/java/eu/stamp_project/testrunner/EntryPoint.java 17 23 73.91%
src/main/java/eu/stamp_project/testrunner/listener/junit5/CoveredTestResultsPerJUnit5TestMethod.java 24 34 70.59%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JacocoRunnerCoveredResultPerTestMethod.java 15 27 55.56%
src/main/java/eu/stamp_project/testrunner/listener/impl/CoveredTestResultPerTestMethodImpl.java 52 71 73.24%
src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveredTestResultsPerJUnit4TestMethod.java 28 110 25.45%
Totals Coverage Status
Change from base Build 956488870: 2.1%
Covered Lines: 1006
Relevant Lines: 2298
💛 - Coveralls

@andre15silva
Fix CoverageFromClass not being serializable.
5c88287 
Because CoverageFromClass was not serializable, loading a serialized
CoverageDetailed object was not possible.

Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Update tests.
40ccab3 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Add documentation
5fce519 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Fix flakyness
f6fb256 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva
Add API documentation in the README
33f6103 
Signed-off-by: André Silva <andre15andre@hotmail.com>
@andre15silva andre15silva mentioned this pull request Jun 24, 2021
Merged
@andre15silva andre15silva marked this pull request as ready for review June 24, 2021 13:34
@andre15silva
Copy link
Copy Markdown
Contributor Author

andre15silva commented Jun 24, 2021 •
edited
Loading

PR ready for review.

I have a pending issue with test-runner's usage in ASSERT-KTH/flacoco#35 (comment), but since this API works in the test-runner's test suite, I'm not sure what is causing it. Any tip appreciated!

danglotb
danglotb approved these changes Jun 24, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@danglotb danglotb left a comment

Choose a reason for hiding this comment

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

Overall the changes look good to me.

However, I have to open question for the future of `test-runner :

  1. I'm asking myself if we could reduce the code redundancies by encapsulating a TestResultImpl in the CoveredTestResultPerTestMethodImpl to avoid a bit the redundancies.
  2. Should we keep CoveragePerTestMethod and Coverage since we have now a richer object for the same "price", i.e. Coverage + TestResult?

I'll merge the changes ASAP the CI is ok.

Thank you for your contributions! Good job

@danglotb
Copy link
Copy Markdown
Member

danglotb commented Jun 24, 2021

You increased the coverage! https://coveralls.io/jobs/82527638 Thank you

@danglotb danglotb merged commit d7dd726 into STAMP-project:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danglotb danglotb danglotb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andre15silva @monperrus @coveralls @danglotb