JUnit runner fix for len(args) > max_args in argfile.safe_args
Review Request #4294 - Created Oct. 11, 2016 and submitted
|benjyw, kwlzn, mateor, nhoward_tw, stuhood, zundel|
There is currently no test for len(args) > max_args in (https://github.com/pantsbuild/pants/blob/27cff2bba343824bc6745b6b03ec940f23e26ebf/src/python/pants/backend/jvm/argfile.py#L37-L55), and all the test cases in the repo have
argfile.safe_args. Whereas the junit runner change adopted
_Testclass, so in the case of
len(args) > max_args, writing
_Test) into a file errors out:... self._run_tests(test_registry, output_dir, coverage) File "/Users/yic/workspace/pants/src/python/pants/backend/jvm/tasks/junit_run.py", line 526, in _run_tests with argfile.safe_args(batch, self.get_options()) as batch_tests: File "/opt/twitter/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/Users/yic/workspace/pants/src/python/pants/backend/jvm/argfile.py", line 53, in safe_args yield create_argfile(fp) File "/Users/yic/workspace/pants/src/python/pants/backend/jvm/argfile.py", line 40, in create_argfile f.write(delimiter.join(args)) Exception message: sequence item 0: expected string or Unicode, _Test found
This change corrects it by making sure string/unicode is passed into the
_execute_junit_runnerto support testing multiple files.
- Add test for
Ship it! I've got some minor comments.
Also, I noticed that safe_args has no tests. I think that's outside the scope of this change, but it'd be nice to have more coverage. Since you're in the area, do you think you have bandwidth to add a couple unit tests as a separate change?
Revision 2 (+105 -37)