Skip to content
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

Update tests with coverage docs with Gradle examples #21896

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

brianwyka
Copy link
Contributor

@brianwyka
Copy link
Contributor Author

I think this needs some more work on my end. Thought I had it working, but maybe not yet... Still seeing some issues when running a combination of plain unit tests and tests with @QuarkusTest annotation

@brianwyka
Copy link
Contributor Author

brianwyka commented Dec 3, 2021

I'm struggling to get the coverage verification to be aligned with the combination of plain unit tests and @QuarkusTest. The report is unified just fine, but the verification seems to be disjoint.

Probably related to #18726

@brianwyka
Copy link
Contributor Author

Perhaps the excludes are not taken into account for the verification. I think that may be the disconnect here.

Copy link
Member

@glefloch glefloch left a comment

Choose a reason for hiding this comment

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

Thanks for this @brianwyka. The overall looks good to me. I left some comments about titles and applying the jacoco plugin

docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
@brianwyka
Copy link
Contributor Author

Thanks for the review @glefloch . I'll make the changes. However, I think the section on the verification is still inaccurate since it does not work correctly at the moment. Is there any idea there?

@brianwyka
Copy link
Contributor Author

I believe I found a workaround to the issue (reported on Gradle):
gradle/gradle#14760

Is it okay to document this workaround until its fixed on Gradle side?

@glefloch
Copy link
Member

glefloch commented Dec 3, 2021

If it's working, I think we can with a reference to the issue

@brianwyka
Copy link
Contributor Author

@glefloch , updated docs.

Copy link
Member

@glefloch glefloch left a comment

Choose a reason for hiding this comment

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

just left few comments. Should be good after that.

docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tests-with-coverage.adoc Outdated Show resolved Hide resolved
@brianwyka
Copy link
Contributor Author

Should be all set now @glefloch

Copy link
Member

@glefloch glefloch left a comment

Choose a reason for hiding this comment

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

Thanks @brianwyka

@gsmet
Copy link
Member

gsmet commented Dec 7, 2021

I force pushed a rebase + squash. Will merge as soon as CI is green.

@glefloch
Copy link
Member

glefloch commented Dec 8, 2021

Thanks @gsmet and @brianwyka !

@glefloch glefloch merged commit 8dbf341 into quarkusio:main Dec 8, 2021
@quarkus-bot quarkus-bot bot added this to the 2.6 - main milestone Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants