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

feat(CONF-CPC-1413): qodana improve coverage #956

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

viktorkuts
Copy link
Collaborator

@viktorkuts viktorkuts commented Oct 17, 2024

JIRA: https://champlainsaintlambert.atlassian.net/browse/CPC-1413

Context

When Qodana fails, the build report is not packed and archived. This PR attempts to run the archive of build reports even if Qodana build fails. We would also want to include Jacoco coverage inside the PR comments.

Changes Github Workflows

Changes

  • Run archive step when Qodana succeeds or fails
  • Bot comments on PR with Jacoco Coverage result (if PR doesn't have any tests, only overall project coverage will be shown)

image

@viktorkuts
Copy link
Collaborator Author

Artifact is created even if qodana fails
image

@viktorkuts viktorkuts marked this pull request as ready for review October 17, 2024 20:01
@viktorkuts viktorkuts changed the title Updated steps to run if failed bug(CONF-CPC-1413): updated steps to run if failed Oct 17, 2024
@nic5694
Copy link
Collaborator

nic5694 commented Oct 17, 2024

i would suggest changing it to a feature it was not a bug. It is normal that the task would not run if the build task has failed. It would only be a bug if you had a condition and that condition was true and it was not running

@viktorkuts viktorkuts changed the title bug(CONF-CPC-1413): updated steps to run if failed feat(CONF-CPC-1413): qodana improve coverage Oct 18, 2024
@viktorkuts viktorkuts marked this pull request as draft October 18, 2024 15:12
Copy link

Comment test

@nic5694
Copy link
Collaborator

nic5694 commented Oct 18, 2024

@viktorkuts Here follow this, i think uploading the HTML file might cause issues, this action will do everything you want: https://github.com/marketplace/actions/jacoco-report

@viktorkuts
Copy link
Collaborator Author

@viktorkuts Here follow this, i think uploading the HTML file might cause issues, this action will do everything you want: https://github.com/marketplace/actions/jacoco-report

Yes I've seen this action, however it uses XML and I wanted to try parsing HTML reports beforehand (didn't really work). I'll enable XML reports, seems to be simpler after experimenting, thanks.

@viktorkuts viktorkuts marked this pull request as ready for review October 19, 2024 19:39
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Repository owner deleted a comment from github-actions bot Oct 19, 2024
Copy link

github-actions bot commented Oct 19, 2024

Jacoco Coverage Report

Overall Project 83.03%
Module Coverage
visits-service-new 91.65% 🍏
Files
Module File Coverage
visits-service-new VisitServiceImpl.java 85.79% 🍏

@viktorkuts
Copy link
Collaborator Author

There is a bug within jacoco-report action, which shows services not meeting the min-coverage as passing. 1.4 should have fixed this, however the feature where we need to use multiple reports is present in the latest release, 1.4 errors.
Here is the issue

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