pantsbuild #development

Channels

welcome

github-notifications

development

happy-kitchen-89482

04/12/2017, 8:42 PM

1. This doesn’t pass

./pants test tests/python/pants_test/backend/python/tasks2:: -- -k test_coverage_paths_option

happy-kitchen-89482

04/12/2017, 8:43 PM

Whereas this does:

./pants test tests/python/pants_test/backend/python/tasks2:: -- -k test_coverage_modules_option

happy-kitchen-89482

04/12/2017, 8:43 PM

[And all other tests for the new pipeline pass]

happy-kitchen-89482

04/12/2017, 8:44 PM

The other issue is that we have to upgrade the old pytest_run task too, similarly to the new one, because we can’t have conflicting versions of pytest in the same pants binary

rough-minister-58256

04/12/2017, 8:45 PM

hm, aren’t those resolved into the test pex at runtime tho?

happy-kitchen-89482

04/12/2017, 8:45 PM

Note that new pytest_cov/coverage changed how they interpret those arguments (as paths vs. modules). They used to use some heuristic re forward slashes, and you could specify an individual

.py

file. Now they just check if the arg is an existing directory on the filesystem, and if not assume the arg is a module.

happy-kitchen-89482

04/12/2017, 8:45 PM

And you cannot specify a

.py

file, only a dir.

happy-kitchen-89482

04/12/2017, 8:45 PM

Hmm, oh, good point.

happy-kitchen-89482

04/12/2017, 8:46 PM

Well, anyway, some of those old pipeline tests fail, so that might require looking into.

happy-kitchen-89482

04/12/2017, 8:46 PM

Or it could be totally unrelated

rough-minister-58256

04/12/2017, 8:46 PM

rgr

happy-kitchen-89482

04/12/2017, 8:46 PM

Maybe they pass now, who knows…

happy-kitchen-89482

04/12/2017, 8:48 PM

The issue seems related to the fact that relative paths are resolved by coverage/pytest_cov in complicated ways that depend on the cwd (at test runtime and maybe also at coverage file read time, I’m not sure), and various path-resolution settings.

happy-kitchen-89482

04/12/2017, 8:49 PM

I was debugging this by printing out coverage reports etc, and noticing that the .coverage file genuinely thinks that there was 0% coverage, when we expect %100.

happy-kitchen-89482

04/12/2017, 8:49 PM

Eventually I got too knotted up to deal with it any further

happy-kitchen-89482

04/12/2017, 8:50 PM

The reason I want to upgrade pytest and friends before tackling the file display issue is because the fix will likely require customizing pytest, and how that works has changed quite a bit.

happy-kitchen-89482

04/12/2017, 8:51 PM

In new pytest we can intercept the call that displays those chrooted files and replace them with their source tree counterparts

happy-kitchen-89482

04/12/2017, 8:52 PM

So, now that I think of it, both issues are to do with pytest doing test discovery in the chroot, but humans wanting to interact via source tree paths.

happy-kitchen-89482

04/12/2017, 8:53 PM

Feel free to reach out if you need help, I wish you better luck than I had… 🙂

rough-minister-58256

04/12/2017, 8:54 PM

will do - thanks 🙂

witty-crayon-22786

04/13/2017, 12:59 AM

@rough-minister-58256 : https://github.com/pantsbuild/pants/pull/4463

rough-minister-58256

04/13/2017, 1:01 AM

nice, 🚢

witty-crayon-22786

04/13/2017, 1:01 AM

...i didn't isort. argh

happy-kitchen-89482

04/14/2017, 12:53 AM

@rough-minister-58256 any luck?

rough-minister-58256

04/14/2017, 12:54 AM

haven’t looked yet

rough-minister-58256

04/14/2017, 9:50 PM

seems like async clean-all is going to bite us again w/ the daemon. that change, man.

witty-crayon-22786

04/14/2017, 9:51 PM

ehhhh?

rough-minister-58256

04/14/2017, 9:54 PM

in a pantsd run, when a new pex is fetched and async clean-all is invoked, the open file`.pants.d/pantsd/pantsd.log` goes away - which presents as an IOError in the top level run afaict

rough-minister-58256

04/14/2017, 9:54 PM

just saw it in passing - will dig into it more

witty-crayon-22786

04/14/2017, 9:55 PM

hm. clean-all kills the daemon i thought