feat: create annotate only from diff coverage

[MartinBernstorff]

Tried my hand here! A few challenges remain, mainly that I'm not used to Jinja and doing web-mocking in Python, so unsure how to fix those test errors :-)

[github-actions]

End-to-end public repo
End-to-end private repo
If the tests fail, @MartinBernstorff will be added to the private repo

[MartinBernstorff]

A few type issues were caught by pyright that mypy seems to have missed - I've chosen to ignore them for now, but do recommend pyright over mypy 👍

https://github.com/microsoft/pyright/blob/main/docs/mypy-comparison.md

[kieferro]

Thank you for the PR :) That looks pretty good

I'm not used to Jinja and doing web-mocking in Python, so unsure how to fix those test errors :-)

One of the tests that is not successful is simply because the output here has changed because line 6 is not part of the DiffCoverage, but part of the original codebase. So that actually proves that the changes work 😄

[kieferro]

With these changes, the two tests in the file are now exactly the same (except for the annotation type). Actually, the point was that the second test uses a fixture with more missing lines in several files. This is what coverage_obj_many_missing_lines was for. Would it be possible to rework this fixture to return a DiffCoverage object so that the test still serves its original function?

[MartinBernstorff]

Ah yeah, that's an excellent point. Is there any reason that the n_lines>2 case is different from the n_lines==2 case in the other test in the file? I've removed it for now, let me know if it's required.

[kieferro]

Is there any reason that the n_lines>2 case is different from the n_lines==2 case in the other test in the file?

This test is mainly for testing a bit more extensively (more files, more missing lines, more edge cases). I have committed a suggestion how I would imagine this.

[MartinBernstorff]

Thanks for an excellent review experience, @kieferro! I'm still not quite sure how tests/integration/test_main.py::test_action__pull_request__annotations creates foo.py, and what git history it uses.

My understanding is that the integration test environment is setup in integration_env. I can see it create files A and B, then switch to another branch and create file C. However, I don't know when it creates foo.py.

get_diff_coverage_info used origin/main as its comparator to get diff coverage. I've added an option to not use origin, but we get no diff from foo.py. When I know where foo.py is created, I might be able to debug it :-)

Let me know what you guys think, and feel free to commit to it as well 👍

[kieferro]

I can see it create files A and B, then switch to another branch and create file C.

These are actually not files, just variables created in foo.py. If you look closely here you can see that file_path is just a fixture and "A", "B" and "C" as parameters are the variables and not the filenames. That means the file foo.py will look like this in the end:

if os.environ.get("A"):
 1
if os.environ.get("B"):
 2
if os.environ.get("C"):
 3

And the creation of foo.py happens indirectly here, by using this fixture to write the code with the variables into it.

I hope that was understandable. If not, feel free to ask, I'm always happy to support with explanations (or code) 😃

Let me know what you guys think, and feel free to commit to it as well 👍

Thanks 👍, I have added two commits with small changes.

[ewjoachim]

Good ! I have some changes I drafted but haven't posted yet on changing the integration fixture so that there is some diff coverage lines missing in the general case (changes the 77% figure in quite a few different test). I guess the best course of action would be to merge as is (when tests pass) and I'll do a follow-up PR with new tests, so as not to delay this too much.

[github-actions]

Coverage report

The coverage rate went from 100% to 100% ➡️
The branch rate is 100%.

100% of new lines are covered.

Diff Coverage details (click to unfold)

coverage_comment/main.py

100% of new lines are covered (100% of the complete file).

coverage_comment/coverage.py

100% of new lines are covered (100% of the complete file).

coverage_comment/annotations.py

100% of new lines are covered (100% of the complete file).

coverage_comment/settings.py

100% of new lines are covered (100% of the complete file).

[ewjoachim]

Congratulations on your first PR here ! Thank you very much :)

[ME-ON1]

Hey i think this has broken the --compare-branch feature

[kieferro]

@MartinBernstorff Unfortunately I had to revert this PR as a quickfix, because it caused the action to not work properly (#192).
That this happened is in no way your fault. We as reviewers are responsible and we should have checked this more carefully before we merge. Nevertheless we would like to have this feature and your contribution in the codebase. (If you want) you are welcome to reopen the same PR from your branch and we'll review again in more detail and hopefully without missing anything.