Skip to content

Conversation

jaskij
Copy link
Contributor

@jaskij jaskij commented Dec 11, 2024

rendered view

As discussed in #398 , this documents how to integrate cargo-llvm-cov with GitLab CI.

As of now, there's several things missing to be resolved, but I wanted to open the PR as a starting point. For now, this is a draft.

  • test coverage rendering
  • splitting the Cobertura report into multiple files to obey GitLab limits
  • which coverage in the summary?

Coverage rendering

This I will do soon, but due to the way GitLab renders only diffed files in MRs/PRs, I did not have an opportunity to test.

Splitting

While it's generally out of scope to describe the whole issue, adding a single sentence mention with a link to GitLab's documentation will probably be the right call.

Which coverage in summary

For now, the regex I'm using will use region coverage - I felt it'd be the most accurate, but I can just as easily extract function or line coverage.


Closes #398

@jaskij
Copy link
Contributor Author

jaskij commented Dec 11, 2024

cc @mike-kfed

@jaskij
Copy link
Contributor Author

jaskij commented Dec 11, 2024

Hrmm... CI tells me I made a spelling mistake, but not where. And I'm not seeing it.

I added that note about report size limits

@mike-kfed
Copy link
Contributor

looks like the markdown lint fails:

README.md:596 MD028/no-blanks-blockquote Blank line inside blockquote
README.md:600:2 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]

seems like tidy.sh runs all checks no matter what and returns an error code if any of them failed.

@jaskij
Copy link
Contributor Author

jaskij commented Dec 12, 2024

Thanks for pointing that out. I fixed the trailing whitespace, but left the empty line as is - it's there on purpose, and I'm explicitly using GitHub Flavored Markdown here. Which, I'm not sure that I should.

In the meantime, I was working on my project using these coverage reports, and can now confirm - they are displayed correctly, although it is only a line-based report.

Overall, I'm fine with how it is now.

@jaskij jaskij marked this pull request as ready for review December 12, 2024 07:37
@taiki-e taiki-e force-pushed the main branch 2 times, most recently from 932a369 to 67a45bf Compare December 18, 2024 19:57
Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

As for limits, I think it is enough to mention gitlab doc for now.

@mike-kfed
Copy link
Contributor

just fyi, I recently made my tools for working around gitlab limitations installable with cargo install lcov2xml. I know it's not part of the scope of this PR, but maybe helpful for future documentation contributions.

@taiki-e
Copy link
Owner

taiki-e commented Jan 9, 2025

@mike-kfed That's great. I'm would accept a PR that adds a mention of your tool after this PR is merged.

@jaskij
Copy link
Contributor Author

jaskij commented Jan 9, 2025

@taiki-e I'm kinda confused with your workflow right now. Do I need to do anything?

@taiki-e
Copy link
Owner

taiki-e commented Jan 10, 2025

CI failure is rustc bug (rust-lang/rust#135235) fixed today. I just re-ran CI and will merge this PR in after it passes.

@taiki-e taiki-e merged commit 73f6618 into taiki-e:main Jan 10, 2025
28 checks passed
@taiki-e taiki-e added the C-documentation Category: related to documentation. label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-documentation Category: related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for GitLab integration?
3 participants