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

update coverage to version 6 #135

Merged
merged 3 commits into from Mar 20, 2022
Merged

Conversation

gro1m
Copy link
Contributor

@gro1m gro1m commented Feb 10, 2022

Might resolve issue: #133

test-requirements.txt Outdated Show resolved Hide resolved
@gro1m
Copy link
Contributor Author

gro1m commented Feb 11, 2022

Hi @aconrad
Do you know why the coverage pipelines are now pending? - thanks for clarifying :)

@aconrad
Copy link
Owner

aconrad commented Feb 11, 2022

Yes, because they are failing to parse the coverage file per my comment here: #133 (comment)

@gro1m
Copy link
Contributor Author

gro1m commented Feb 11, 2022

ah ok...

@gro1m
Copy link
Contributor Author

gro1m commented Feb 15, 2022

@aconrad Does this comment have any relevance for us?
nedbat/coveragepy#578 (comment)

Or do we really need this PR?
nedbat/coveragepy#1146

@aconrad
Copy link
Owner

aconrad commented Mar 3, 2022

@aconrad Does this comment have any relevance for us? nedbat/coveragepy#578 (comment)

It does, and I commented in this issue.

Or do we really need this PR? nedbat/coveragepy#1146

Hm, I don't know. I'm not intimately familiar with coveragepy. But maybe? Have you tried to checkout that branch and see if it solves the issue? If yes, we could try to push for the PR to get merged...

@gro1m
Copy link
Contributor Author

gro1m commented Mar 6, 2022

Hi @aconrad

Or do we really need this PR? nedbat/coveragepy#1146

Hm, I don't know. I'm not intimately familiar with coveragepy. But maybe? Have you tried to checkout that branch and see if it solves the issue? If yes, we could try to push for the PR to get merged...

Does not change anything in my opinion, but you can check yourself with:

git+https://github.com/alex/coveragepy.git@patch-1 
mock
pytest-cov==3.*

in the test-requirements.txt file.
Beware though that there is a small caveat:
The tox.ini should actually use --force-reinstall for the pip installs, so that every time you rebuild locally the package versions are taken from the test-requirements.txt even if they already exist.

@gro1m
Copy link
Contributor Author

gro1m commented Mar 6, 2022

So how can we apply nedbat/coveragepy#578 (comment)?
How can we switch from the source option to the include option, as I do not see yet where the .coveragerc comes into play in pycobertura...

@aconrad
Copy link
Owner

aconrad commented Mar 7, 2022

So how can we apply nedbat/coveragepy#578 (comment)?
How can we switch from the source option to the include option, as I do not see yet where the .coveragerc comes into play in pycobertura...

Unfortunately, this comment is about generating cobertura files when the code coverage occurs (during test runs). We only parse them.

@aconrad
Copy link
Owner

aconrad commented Mar 7, 2022

Hm, wait. I guess we do generate our own coverage.xml files. We can try to change it the way they recommend it in the comment for the purpose of getting the coverage of this branch to pass in our own repository if that was your intent. At least that would be a thing to improve!

@aconrad
Copy link
Owner

aconrad commented Mar 7, 2022

The coveragepy documentation says that it will look for configuration in a tox.ini file, if present, and parse sections of it that are prefixed with [coverage:...], such as [coverage:run].

I tried different combinations to see if it affects the output of coverage.xml but no dice, I wasn't able to make anything happen. The doc says that include = ... will be ignored if there is a source = ... present. When I pass include = ..., I get a warning saying that it's being ignored because source is present even though it's not. So I'm not sure where it's picking up the source from...

I wasn't able to make further progress that.

@gro1m
Copy link
Contributor Author

gro1m commented Mar 12, 2022

Hi @aconrad Should be fixed (I tested locally with Python 3.9) with the most recent commit: ed261ce.
But I do not understand why no pipeline is getting triggered ?!

@gro1m
Copy link
Contributor Author

gro1m commented Mar 12, 2022

And also could you maybe somehow stop the pending pipelines... :)

@aconrad
Copy link
Owner

aconrad commented Mar 12, 2022

Oh boy. Travis is saying we have insufficient credit again which is why it's not running. That's it, we're moving to GitHub Actions. I'll have to work on that in the next few days. Sorry for the inconvenience.

@aconrad
Copy link
Owner

aconrad commented Mar 12, 2022

And thanks for working on this! I don't know why .coveragerc works but not tox.ini per the documentation. Great job!

@gro1m
Copy link
Contributor Author

gro1m commented Mar 13, 2022

Oh boy. Travis is saying we have insufficient credit again which is why it's not running. That's it, we're moving to GitHub Actions. I'll have to work on that in the next few days. Sorry for the inconvenience.

This might also allow you to populate the Releases Tab (not only the tags) :)

@aconrad
Copy link
Owner

aconrad commented Mar 16, 2022

Sounds good. I'm very busy with work this week, not sure I'll have time to take care of this right this week.

@aconrad aconrad merged commit 0874a31 into aconrad:master Mar 20, 2022
@aconrad
Copy link
Owner

aconrad commented Mar 21, 2022

Should be fixed (I tested locally with Python 3.9) with the most recent commit: ed261ce.

For me, the coverage.xml is still generated without the pycobertura/ prefix directory. I get the following warning:

venv/lib/python3.8/site-packages/coverage/inorout.py:472
  /Users/alexandre/Development/pycobertura/venv/lib/python3.8/site-packages/coverage/inorout.py:472: CoverageWarning: --include is ignored because --source is set (include-ignored)

Don't you get this error?

This is the same message I got when I fiddled with adding [coverage:run] inside tox.ini.

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

Should be fixed (I tested locally with Python 3.9) with the most recent commit: ed261ce.

For me, the coverage.xml is still generated without the pycobertura/ prefix directory. I get the following warning:

venv/lib/python3.8/site-packages/coverage/inorout.py:472
  /Users/alexandre/Development/pycobertura/venv/lib/python3.8/site-packages/coverage/inorout.py:472: CoverageWarning: --include is ignored because --source is set (include-ignored)

Don't you get this error?

This is the same message I got when I fiddled with adding [coverage:run] inside tox.ini.

Yes with pytest I actually do get that error message as well, but the output is still rendered correctly. Whatever that means...

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

With pycobertura show coverage.xml on the other hand I do not get the error.
I need to correct this statement: I do not get the warning, but I also do not get the prefix. This is strange...

With pytest I get the warning, but the output is rendered correctly.

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

Ah, I think this is because with pycobertura show coverage.xml the .coveragerc is probably not taken into account...

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

And is not source actually a parameter:
https://github.com/aconrad/pycobertura/blob/master/pycobertura/cli.py#L92-L101

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

@aconrad Ok, the problem is that we have a setup.cfg pytest section (in addition to tox.ini). It is recommended to have it in tox.ini (see also: https://docs.pytest.org/en/6.2.x/customize.html). Just removing it, unfortunately does not solve the problem...

@gro1m
Copy link
Contributor Author

gro1m commented Mar 21, 2022

@aconrad Should be solved with: #140. Note that apparently all coverage related stuff has to be in the [testenv] and not in the general [pytest] configuration section. That should solve the problem :)

@aconrad
Copy link
Owner

aconrad commented Mar 22, 2022

Oh awesome! Thanks! I'll test it on my machine tomorrow...

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.

None yet

2 participants