When source fields are strings, not collections, raise an error; Test deferred sources addresses error

Review Request #3970 — Created June 3, 2016 and submitted

nhoward_tw
pants
3507, 3550
pants-reviews
benjyw, jsirois, kwlzn, zundel

Currently if a string is passed to sources in a target definition, it will be treated as a list of single character filenames, which blows up in a confusing way later. This makes passing a string an error.

While I was in here, I added unit tests covering the too many addresses case for deferred sources and uncovered a bug where if the address kwarg isn't passed, instead of raising WrongNumberOfAddresses, it would instead hit an AttributeError on None.

Wrote a few regression tests and made them pass.

Jenkins and travis passed

  • http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3550/1/
  • https://travis-ci.org/pantsbuild/pants/builds/135065849
NH
  1. 
      
  2. src/python/pants/build_graph/target.py (Diff revision 1)
     
     

    I could use address.spec here if it's available, but I decided not to because few/no callsites use it.

  3. 
      
BE
  1. Ship It!
  2. 
      
KW
  1. lgtm. would it make sense to also fix this for the v2 engine as well?

    with this RB patched in, I noticed that ./pants list bad_target fails gracefully as intended while ./pants --enable-v2-engine list bad_target silently succeeds. it'd be great to aim for parity between these two modes for fixes going forward.

  2. 
      
ZU
  1. Ship It!
  2. 
      
NH
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as https://github.com/pantsbuild/pants/commit/4b58f02549d400cb9ed95039230eb607b3d1a62d
Loading...