Move a comment to work around a pytest bug.

Review Request #2201 — Created May 11, 2015 and submitted

benjyw
pants
1525
c888596...
pants-reviews
jsirois, zundel

When that test fails, pytest attempts to construct a verbose backtrace.
However that comment in the middle of a multiline list literal tickles
a bug in the backtrace construction code that causes pytest to fail hard
with an INTERNALERROR, instead of showing the details of the underlying bug.

While debugging this, I noticed that we don't constrain the versions of
pytest that we bake into the test chroot, so it will end up being a
different version than the one in 3rdparty/python/requirements.txt.
This is pretty confusing, so I added the relevant constraints.

See here for an example of this useless INTERNALERROR dump:
https://travis-ci.org/pantsbuild/pants/jobs/62120623

Relevant test passes locally in otherwise unmodified repo.
Relevant test fails with INTERNALERROR when test is deliberately broken before this change.
Relevant test fails with useful backtrace when deliberately broken after this change.

CI pending: https://travis-ci.org/pantsbuild/pants/builds/62150849

JS
  1. 
      
  2. An install need not have 3rdparty requirements for any of these in which case its not so confusing.  I'm in favor of pinning what we support though for now.  I think the right thing TODO is as you suggest - ie: add config options for overriding these specs so users need not wait on a pants release to use that new pytest feature they want.
    1. That's a good point. It's confusing to pants devs, but not to the public at large. I will rephrase the TODO to reflect making these options.

  3. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as d20a43faebe7b9c173edefd3940bdf96342f1fb4.

BE
  1. Thanks John! Submitted as d20a43faebe7b9c173edefd3940bdf96342f1fb4.

  2. 
      
ZU
  1. Ship It!
  2. 
      
Loading...