JUnit runner fix for len(args) > max_args in argfile.safe_args

Review Request #4294 — Created Oct. 11, 2016 and submitted

wisechengyi
pants
pants-reviews
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 args passing through argfile.safe_args. Whereas the junit runner change adopted _Test class, so in the case of len(args) > max_args, writing args (list of _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 argfile.safe_args function.

Other changes:

  • Privatize _execute_junit_runner.
  • Change _execute_junit_runner to support testing multiple files.
  • Add test for argfile.safe_args.

https://travis-ci.org/pantsbuild/pants/builds/167164195

WI
NH
  1. 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?

  2. Could you destructure t here into something like file_name, file_content?

  3. nit: s/junt/junit/

    Also, maybe call it test_junit_run_with_too_many_args?

  4. How about [file_name for file_name, _ in ....

  5. 
      
BE
  1. Ship It!
  2. 
      
MA
  1. Ship It!
  2. 
      
ST
  1. Ship It!
  2. 
      
WI
WI
Review request changed

Status: Closed (submitted)

Change Summary:

df21f68cc6ddff6814e28ad66db3eb337c3aa206

Loading...