Re-add the ConsoleRunnerOutputTests and consolodate them into ConsoleRunnerTest, also move test clases used for testing into junit/lib directory

Review Request #3588 — Created March 18, 2016 and submitted

cheister
pants
readd-console-runner-command-line-tests
3067
pants-reviews
gmalmquist, patricklaw, stuhood, zundel
Re-add the ConsoleRunnerOutputTests and consolodate them into ConsoleRunnerTest, also move test clases used for testing into junit/lib directory

Travis CI: https://travis-ci.org/pantsbuild/pants/builds/117841276

This re-adds most of the changes from https://rbcommons.com/s/twitter/r/2406/ to make the swappable streams non-static in ConsoleRunnerImpl and testable.

It also moves all the Test classes that are only used to test the ConsoleRunnerImpl into src/java/org/pantsbuild/tools/junit/lib so they are under a java_library() target and do not get run as regular tests under junit/impl. It updates the java packages so they match the directory the are in.

Finally, it renames ConsoleRunnerTestHelper to ConsoleRunnerTestBase and consolodates the ConsoleRunnerOutputTests into ConsoleRunnerTest.

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
  1. Just to be clear, there was something quite wrong here before: a directory named 'tests/java/org/pantsbuild/tools/junit/lib' but the package declared inside was 'tests/java/org/pantsbuild/tools/junit/impl' which was very confusing (and I'm surprised it worked at all)

    Chris ironed this out by separating out which tests actually needed to be executed in the junit_tests() target and moved the rest into the lib directory and updated the package. Then he also sprinked in some more cleanups and improved tests.

    1. @Chris: can you include some of the above in the description?

  2. many cooks have been in here, thanks for making this code adhere more to convention.

  3. thank you for these additional tests

  4. Update this comment. Now this test isn't run from a pants BUILD target, its only executed through ConsoleRunnerTest, so there's no need to turn off the flaky behavior.

  5. Something to note in all the tests you moved is that these tests aren't run directly from a junit_tests() BUILD target, they are referenced through specific invocations in ConsoleRunnerTest. That's pretty unusual, you might want to note this special situation in the tests/java/org/pantsbuild/tools/junit/lib/BUILD file.

    1. I added the following header to each of the Tests that are not run.

      /**
       * This test is intentionally under a java_library() BUILD target so it will not be run
       * on its own. It is run by the ConsoleRunnerTest suite to test ConsoleRunnerImpl.
       */
      
    2. We could also consolodate the ConsoleRunnerTestBase and ConsoleRunnerTest classes since there is only one test class now, but I'm going to leave the ConsoleRunnerTestBase class incase we decide to split the tests up later.

  6. 
      
  1. Ship It!
  2. 
      
  1. Ship It!
  2. 
      
  1. Thanks Stu for the review and Chris for the patch. This is now in master @ 3c0c537. Please close and mark this review as submitted and close the associated github PRs.

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...