search

aantron / bisect_ppx

Code coverage for OCaml and ReScript
http://aantron.github.io/bisect_ppx/demo/
MIT License
302 stars 60 forks source link

Report Cobertura XML format #383

Closed vch9 closed 3 years ago

vch9 commented 3 years ago

I'm creating this PR a bit early to gather your views on this @aantron. This is the minimal working version of reporting to Cobertura XML.

~~Some fields in type cobertura such as branches_covered, branches_valid etc. might be removed in the near future. I took inspiration from examples linked in #326 (e.g. this) and I need to see which fields are mandatory.~~ Removed, they were useless.

On top of the commit 352a5d152a2979be95e254f12db454cec07b0a98, I produced the report with the rule cobertura and used it in this diff.

Closes #326 (I'm closing a closed issue :p)

vch9 commented 3 years ago

I removed several useless fields in the report. This diff uses the minimal information about lines covered and rates.

The PR is then ready for review.

vch9 commented 3 years ago

The code looks basically good. What are you using to view (render) the Cobertura reports?

I am using directly the cobertura report in GitLab:

stages:
  - test

variables:
  coverage_report_view: "true"

coverage:
  stage: test
  script:
  - echo \"making the coverage\"
  artifacts:
    paths:
      - ./coverage/cobertura-coverage.xml
    reports:
      cobertura: coverage/cobertura-coverage.xml

Then, I push the cobertura generated by bisect in a MR: https://gitlab.com/vch9/corbetura-reports/-/merge_requests/5/diffs

(If you don't see any colors; you should refresh the page)

aantron commented 3 years ago

I am using directly the cobertura report in GitLab:

Is this rendered somewhere on GitLab, that you could share a link to? I wasn't able to find it in a few minutes. I do see the MR diffs, pipelines, even raw coverage reports, but not rendered coverage reports.

vch9 commented 3 years ago

I am using directly the cobertura report in GitLab:

Is this rendered somewhere on GitLab, that you could share a link to? I wasn't able to find it in a few minutes. I do see the MR diffs, pipelines, even raw coverage reports, but not rendered coverage reports.

The rendered coverage reports is directly inside the diffs (as in GitLab documentation). Do you expect another form of rendered coverage reports or you just don't see the coverage in diffs?

image

aantron commented 3 years ago

I actually don't see it:

Снимок

Perhaps it's a users' permission issue.

Thanks for the screenshot, though, it's what I was looking for :) I'll probably be using GitLab then whenever I am testing changes to this code in the future.

vch9 commented 3 years ago

Perhaps it's a users' permission issue.

That's weird, even if I open in another browser (not logged in) I can see them. Can you try to refresh the page? Sometimes the colors appear after a couple refresh (lost a couple hours on this).

Thanks for the screenshot, though, it's what I was looking for :) I'll probably be using GitLab then whenever I am testing changes to this code in the future.

That is pretty complicated but I did not manage at this point a better way to test the format :/

aantron commented 3 years ago

Can you try to refresh the page?

Ah, it's working now after some refershes. I didn't know if that's what you originally meant by refreshing if I don't see any colors, because I saw the regular diff colors and didn't know if there were more I should be looking for :) Thanks!

aantron commented 3 years ago

Thank you! This looks good to me!

Do you want to do anything else to it in this PR? Or may I merge it? :)

vch9 commented 3 years ago

Thank you! This looks good to me!

Thank you for your quick review!

Do you want to do anything else to it in this PR? Or may I merge it? :)

If you don't care about the commits and squash it all you can merge this now :)

vch9 commented 3 years ago

However, we are currently integrating this report in the Tezos CI, maybe we can wait for a large-scale test on Tezos before merging this. Alternatively, I can make a follow-up MR if I find flaws.

aantron commented 3 years ago

I am merging this now, as it does some refactoring, and it's best not to leave such PRs hanging. I'll be happy to review and merge any follow-on PRs :) Thanks!

Note I had to avoid a few functions (Option.fold, List.filter_map, Float.of_int) to get this PR to be compatible with 4.04.2, which is the lowest OCaml version Bisect still supports. Normally this would be caught by CI, but the Travis CI has been deprecated by Travis. I hope to soon get the replacement GitHub Actions CI fully testing everything.

aantron commented 3 years ago

FYI I'm doing a bunch of refactoring in src/report/ (for the first time since taking over the repo). If you've started doing any work based on a commit before the refactoring, feel free to open a PR without rebasing. I'm willing to apply the changes I am making to the repo to any PR, as well.

vch9 commented 3 years ago

Note I had to avoid a few functions (Option.fold, List.filter_map, Float.of_int) to get this PR to be compatible with 4.04.2, which is the lowest OCaml version Bisect still supports. Normally this would be caught by CI, but the Travis CI has been deprecated by Travis. I hope to soon get the replacement GitHub Actions CI fully testing everything.

Thank you! I'll pay attention next time and work with a dedicated switch.

FYI I'm doing a bunch of refactoring in src/report/ (for the first time since taking over the repo). If you've started doing any work based on a commit before the refactoring, feel free to open a PR without rebasing. I'm willing to apply the changes I am making to the repo to any PR, as well.

I started to integrate this report in our project and it's working fine for now, there should not be changes in the near future. Thank you though for the notice :)