Page MenuHomePhabricator

Fix unit test coverage for `NoseTestEngine`.
ClosedPublic

Authored by joshuaspence on Mar 7 2014, 4:21 AM.
Tags
None
Referenced Files
F14033770: D8433.diff
Sat, Nov 9, 7:59 PM
F14004309: D8433.id20033.diff
Sat, Oct 26, 6:18 PM
F13994487: D8433.diff
Wed, Oct 23, 6:30 AM
F13962544: D8433.id20017.diff
Tue, Oct 15, 10:43 AM
Unknown Object (File)
Sep 26 2024, 4:40 PM
Unknown Object (File)
Sep 16 2024, 4:27 AM
Unknown Object (File)
Aug 29 2024, 7:16 PM
Unknown Object (File)
Aug 22 2024, 8:16 PM

Details

Summary

I noticed that code coverage wasn't showing in Differential for
some repositories that we are using with Phabricator.

arc unit would should unit test coverage, but the paths were messy
(for example, .//foo/bar.py instead of foo/bar.py). As a result,
the code coverage info wasn't recognised as being for the correct
module.

I'm not sure why this logic is the way that it is... perhaps this is to
do with an older version of nose (I am using v1.3.0).

Test Plan

I created a diff for an internal repository that we have, and
observed that code coverage was displayed in Differential.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Adding some people who might have a better understanding of this than I do, but this seems sensible to me. If this actually breaks anything, give me a quick example of how to reproduce the break and we can get it covered in tests (we have far more ability to do things like mock fake projects now than we probably did when this stuff was written).

epriestley edited edge metadata.
This revision is now accepted and ready to land.Mar 7 2014, 6:15 PM
src/unit/engine/NoseTestEngine.php
133

Oh, I'm going to change this to:

phutil_split_lines(Filesystem::readFile($path))

...which should work better in more cases and raise clearer errors in cases where it doesn't work.