Run coverage in subdirectory

[ewjoachim]

Closes #167 (and linked to #16)

I'm not 100% sure it works, because I've mostly make tests pass, but I'd like to try that for real (and wondering about changing the e2e tests to use a subdir)

[github-actions]

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

[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/files.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/main.py

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

coverage_comment/subprocess.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).

[kieferro]

This looks great <3. That's pretty much how I imagined it too.

I'm not 100% sure it works, because I've mostly make tests pass, but I'd like to try that for real (and wondering about changing the e2e tests to use a subdir)

Yes, that wouldn't be a bad idea at least once for testing, maybe permanently.
Does that mean you are still working on the PR or should I already review it in detail or assist somehow in regards to the e2e tests?

[ludelafo]

Looking forward for this contribution! I just stumbled upon this project and was searching how to set a subdirectory for coverage in my monorepo project.

[ewjoachim]

Yes, that wouldn't be a bad idea at least once for testing, maybe permanently.

I did a one-off test in https://github.com/ewjoachim/python-coverage-comment-action-devenv/tree/master where coverage ran from the src folder and it did work. I guess we're good to go (when you say so @kieferro and when I've solved conflicts)

[ewjoachim]

Coverage report

The coverage rate went from 100% to 99.87% arrow_down The branch rate is 100%.

96.15% of new lines are covered.
Diff Coverage details (click to unfold)

Also, that.

[kieferro]

Looks very good, thank you. Just some small comments from my side. I also took the liberty to fix a few spelling mistakes (that were already there before) and raise the coverage to 100%.

[ewjoachim]

Okay, hopefully, we should be good :)

[kieferro]

Okay, that looks good so far. However, I did some more tests with a copy of our example repo and for push everything works, but for PR the action crashes. This is because diff-cover always specifies the file paths relative to the root of the repo no matter where you run the command and the other coverage data is always relative to the COVERAGE_PATH.
I would therefore recommend that we always add COVERAGE_PATH to the file paths that are stored in the coverage data. This would also solve another problem. With the change in this PR it is no longer necessarily apparent to which files the comment refers, because absolute paths are no longer used. This can be very confusing, especially when several comments are posted (which will probably happen with #213), because you would have to figure out from the context which code it is about.

It might also be useful to add an integration test that completely tests a case with COVERAGE_PATH once. This way we would probably have noticed the problem earlier.
But I just tried to write such a test myself and I already see that it is not easy if you don't want to duplicate all fixtures. I'm still trying things out a bit. But if you have an idea in that direction, feel free to implement it.

[ewjoachim]

Good news everyone: I've set up the e2e tests to use a src repo, and hopefully, this will reproduce the issue (though I'm still struggling a little)

Also, it's very tedious to launch the action locally :( And I don't reproduce locally :( but I've reproduced on a dev-env GH repo.

[ewjoachim]

Dammit, I don't reproduce the issue :(

[ewjoachim]

There seem to be an awful lot of unrelated commits in this PR :/ Sorry about that, tell me if you'd rather I'd move them in a different one.

[kieferro]

There seem to be an awful lot of unrelated commits in this PR :/ Sorry about that, tell me if you'd rather I'd move them in a different one.

Thank you, very considerate. But it wasn't that bad.

Now everything looks perfect. I also tested it again in a repo, but with the e2e tests (thanks again for all the work you put in to reproduce the issue) it shouldn't be a problem anymore anyway. I guess we should be ready to merge then, right?

[czesiu89]

Hey :)
I just wanted to say that I love the project, it's minimalism in regards of output look etc., but I can't use it without relative paths being merged (monorepo... :)). Are there any time frames on when it will be merged and released :)?

[ewjoachim]

Are there any time frames on when it will be merged and released :)?

I'd say "days", no promises.