Modernize the protobuf and wire task tests

Review Request #1854 — Created March 3, 2015 and submitted

zundel
pants
zundel/modernize-proto-and-wire-task-tests
1194
773e071...
pants-reviews
benjyw, jsirois
  • Get rid of odd inheritance in WireGen test
  • Cleanup BUILD deps
  • Inherit from TaskTestBase instead of TaskTest
  • Refactor alias_groups() to use shortcuts directly from register.py
  • Use self.create_file() helper function
  • Refactor some top level symbols back into the task objects
  • Add test of _calculate_sources that uses the instantiated task
  • Add test for check_duplicate_conflicting_protos

 src/python/pants/backend/codegen/tasks/protobuf_gen.py             |  45 +++++++++---------
 src/python/pants/backend/codegen/tasks/wire_gen.py                 |  46 +++++++++---------
 tests/python/pants_test/backend/codegen/tasks/BUILD                |  13 ++++--
 tests/python/pants_test/backend/codegen/tasks/test_protobuf_gen.py | 262 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 tests/python/pants_test/backend/codegen/tasks/test_wire_gen.py     |  66 ++++++++++++++++++++++----
 5 files changed, 288 insertions(+), 144 deletions(-)

CI is running @ https://travis-ci.org/pantsbuild/pants/builds/52902539

  • 0
  • 0
  • 1
  • 2
  • 3
Description From Last Updated
ZU
  1. For some reason, I have the belief that we are trying to phase out TaskTest in favor of TaskTestBase. I was starting a new test and went back to check on some existing tests I have been maintaining and they looked icky. Are these changes in line with the direction we want task tests to be going?

    1. Your belief is correct. TaskTestBase was created specifically to make it easy to test with new-style options, setting them programmatically and completely bypassing cmd-line parsing and config parsing. TaskTest subclasses have to create fake flags and fake config files. So thanks for fixing this up.

  2. 
      
BE
  1. 
      
  2. The base class already gives you a workdir that gets set up and torn down appropriately.

    1. Thanks for reviewing. That was like picking at a thread on a sweater, a bunch of stuff came unraveled as I fixed that and I ended up changing more code that I originally intended to when I first wrote this RB.

  3. 
      
ZU
BE
  1. 
      
  2. Are you using safe_mkdir?

    1. See: test_protos_extracted_under_build_root line 235

  3. Where is create_file() defined? Looks like it should be on TaskTestBase, but I don't see it there.

    1. That is defined in BaseTest (base_test.py)

  4. 
      
ZU
  1. 
      
  2. See: test_protos_extracted_under_build_root line 235

  3. 
      
BE
  1. Ship It!
  2. 
      
ZU
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks for the review Benjy. Commit d97b02c

Loading...