Fix, document, or mark xfail tests that fail in Jenkins.

Review Request #3632 — Created March 31, 2016 and submitted

Patrick Lawson
pants
3123
pants-reviews
jsirois, mateor, peiyu, stuhood, zundel

The junit tests fail when Java 8 is available,
but not the default. These were likely not running at all in
Travis, but Jenkins has both JDK7 and 8 available, so it exposed
the issues.

The distribution integration test was failing because the distribution
home path was a symlink. realpathing the directory in the test util
code fixes the issue.

The export integration test makes assumptions about subinvocations
of pants that are not correct. In particular, it assumes that the
default ivy cache path will be used. This breaks when
PANTS_IVY_CACHE_DIR is set. I left a comment indicating as much.

CI is green: https://travis-ci.org/pantsbuild/pants/builds/120139430

  • 0
  • 0
  • 0
  • 2
  • 2
Description From Last Updated
Stu Hood
  1. 
      
  2. Can you include a github issue number here?

  3. Ditto github issue.

  4. Same github issue as the first one?

  5. 
      
Mateo Rodriguez
  1. Ship It!
  2. 
      
Patrick Lawson
Eric Ayers
  1. 
      
  2. Could we further conditionalize this so that maybe we have a setting that could be local to the CI builder to alert us to skip these tests? we depend on this logic and these tests work on Travis and other environments. Now they are just neutered.

    1. I claim these tests do not work on Travis--they don't run at all because of the skipIf and a lack of Java 8 in the Travis environment.

    2. ok, I think you are right but they do succeed on my laptop. If we could just check the presence of a file on disk or an environment variable setting that would be fine for me.

    3. Do they succeed or skip? I'm forcing these failures now on Jenkins since everything else seems to be smoothed out.

    4. Okay, it looks like this was just a symlink issue I already knew about--but I'd mistakenly thought the fix was already upstream. The solution is to just realpath the distribution home in the test code. Updated the RB.

  3. We rely heavily on the export command

    Is there no way we can conditionalize this test? @skipIf(nonstandard_ivy_cache())...

    1. Is this even possible? At global scope, can I access both an option's value and what its default otherwise would have been?

      It's possible that I can skirt around the export tests by not overriding the ivy cache location--that might not be necessary in Jenkins once the locking fix lands.

    2. Looks like this isn't needed with new locking. Dropping it from the change.

  4. 
      
Patrick Lawson
Patrick Lawson
Patrick Lawson
Eric Ayers
  1. 
      
  2. you removed @exepctedFailure, do we still need this comment?

    1. Yes, we do. The test is still broken in that circumstance; it just happens that with new ivy locking we don't need to set PANTS_IVY_CACHE_DIR anymore.

  3. you removed @expectedFailure, do we still need this comment?

  4. 
      
Patrick Lawson
Stu Hood
  1. Ship It!
  2. 
      
Patrick Lawson
Peiyu Wang
  1. already submitted, please close this rb

  2. can you point me to where PANTS_IVY_CACHE_DIR is used? for some reason I couldn't find, there are some very old code by googling

    1. We have PANTS_IVY_CACHE_DIR set in a couple scripts internally. We just export the value - we expect Pants to do something with it. It used to be an explicitly set env_var but Eric removed code reference to this variable a year ago: 26628461bff6856792f49ba8fcbe6375d81e9f25.

      But this is in fact being interpreted by the options system. It is a valid way of passing an arbitrary option - in this case the --cache-dir option within the ivy scope. You can see that this is true in OSS pants by passing it a value, say PANTS_IVY_CACHE_DIR=/home/mateo/not/pants. You will see:

      FAILURE: Classpath entry /Users/mateo/.ivy2/pants/com.squareup.testing.protolib/protolib-test/jars/protolib-test-1.0.1.jar for target 3rdparty:protobuf-test-import is located outside the working directory "/Users/mateo/dev/pants/.pants.d".
      

      This shows that the PANTS_IVY_CACHE_DIR is being parsed by the options system. But it also shows that the classpath construction (or perhaps something earlier in the pipeline) does not respect it. This is likely due to something in the classpath pipeline not using IvyTaskMixin.ivy_cache_dir or perhaps IvySubsystem.global_instance().get_options().cache_dir when setting the cachedir. Those methods will both return the value /home/mateo/not/pants.

      So the --ivy-cache-dir option can be set in this way - but that option appears to be broken. You can pass --ivy-cache-dir to get the same result, and it does not seem to matter if the value is under the workdir or not. Foursquare does set that flag - but since we do not use ivy for resolution we have not been caught by the bug.

  3. 
      
Patrick Lawson
Review request changed

Status: Closed (submitted)

Loading...