search

codeclimate / python-test-reporter

DEPRECATED Uploads Python test coverage data to Code Climate
https://codeclimate.com
Other
19 stars 11 forks source link

Updating for coverage v4.4 #41

Closed jonrkarr closed 7 years ago

jonrkarr commented 7 years ago

Coverage v4.4 changes how file names are reported in XML report files. For example

<class ... filename="karr_lab_build_utils/init.py" ... name="init.py">

is changed to

<class ... filename="init.py" ... name="init.py">

These commits update the test reporter to accommodate this change. The changes are compatible with v4.4 as well as earlier versions.

dblandin commented 7 years ago

Hey @jonrkarr!

This PR looks great. Thanks for the fix!

Would you mind addressing the Code Climate issues? I'd be fine with removing the inline comments. The added lines seem sufficiently intention-revealing to me.

Is the new version of 0.2.1a0 considered an alpha release by pip? Since we'll release a new version, would 0.2.2 or 0.2.2a0 be more appropriate? In either case, we usually bump versions in separate PRs to keep the version history independent from changes merged into master.

jonrkarr commented 7 years ago

Hi Devon,

After I created the pull request I noticed there's already an issue about this. It would be best not to require a specific version of coverage because that would preclude the use of future versions. The coveragepy discussion sounds like the original author isn't going to revert the change that broke python-test-reporter, so it sounds like the solution is to generate complete file paths by combining the sources/source paths and the paths in packages/package/classes/class. For cases with multiple source paths, the author author will need to add an id to sources/source so they can be linked to specific classes and file paths can be unambiguously be resolved.

The changes are only affect a couple lines. Feel free to incorporate the changes as you best see fit.

Regards Jonathan

On Thu, May 11, 2017 at 1:05 PM, Devon Blandin notifications@github.com wrote:

Hey @jonrkarr https://github.com/jonrkarr!

This PR looks great. Thanks for the fix!

Would you mind addressing the Code Climate issues? I'd be fine with removing the inline comments. The added lines seem sufficiently intention-revealing to me.

Is the new version of 0.2.1a0 considered an alpha release by pip? Since we'll release a new version, would 0.2.2 or 0.2.2a0 be more appropriate? In either case, we usually bump versions in separate PRs to keep the version history independent from changes merged into master.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codeclimate/python-test-reporter/pull/41#issuecomment-300853772, or mute the thread https://github.com/notifications/unsubscribe-auth/ACt2KXiKi2aRue8pvyTxXhKJDfQ353XRks5r4z_FgaJpZM4NYQSd .

dblandin commented 7 years ago

For cases with multiple source paths, the author author will need to add an id to sources/source so they can be linked to specific classes and file paths can be unambiguously be resolved.

That sounds like an important addition to make before supporting coverage v4.4 . Would you suggest locking down to <4.4 in the meantime while we figure that out?

We're also working on a new unified test reporter which will support python so I'm less concerned with the long-term maintainability of this particular repo.

https://github.com/codeclimate/test-reporter

jonrkarr commented 7 years ago

Using a specific version of coverage is fine for us for now.

I didn't read the coveragepy discussion until after I made the changes. Had I read that first, I would have just specified a specific version for now until coveragepy sorts out this issue.

On Thu, May 11, 2017 at 2:55 PM, Devon Blandin notifications@github.com wrote:

For cases with multiple source paths, the author author will need to add an id to sources/source so they can be linked to specific classes and file paths can be unambiguously be resolved.

That sounds like an important addition to make before supporting coverage v4.4 . Would you suggest locking down to <4.4 in the meantime while we figure that out?

We're also working on a new unified test reporter which will support python so I'm less concerned with the long-term maintainability of this particular repo.

https://github.com/codeclimate/test-reporter

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codeclimate/python-test-reporter/pull/41#issuecomment-300884367, or mute the thread https://github.com/notifications/unsubscribe-auth/ACt2KaopnQZKjP0sqkH8IWE27L9QXx_Bks5r41mugaJpZM4NYQSd .

dblandin commented 7 years ago

Had I read that first, I would have just specified a specific version for now until coveragepy sorts out this issue.

Yep, that sounds reasonable to me too. Going to close this PR for now.

Thanks!

jonrkarr commented 7 years ago

Thanks for the quick response on this!

On Thu, May 11, 2017 at 3:05 PM, Devon Blandin notifications@github.com wrote:

Had I read that first, I would have just specified a specific version for now until coveragepy sorts out this issue.

Yep, that sounds reasonable to me too. Going to close this PR for now.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codeclimate/python-test-reporter/pull/41#issuecomment-300887093, or mute the thread https://github.com/notifications/unsubscribe-auth/ACt2KUGSoHUyaI_zEgWzrT-MNcLAM2C3ks5r41wHgaJpZM4NYQSd .