Fix test caching behavior of Buck.

Summary:
Since we do not zip the test output with the TestRule's output jar,
we would incorrectly report test results if we have a cache hit for
the rule's output.

CachedTestIntegrationTest was supposed to catch this, but it had a bug
where it was always running tests regardless, since it was looking for the
xmls in the actual source directory instead of the JUnit temp directory.

To fix this, I write the rule key of the rule to it's output directory, and
read it later to validate the output against the current rule key while
checking for cached outputs.

Test Plan:
CachedTestIntegrationTest is green.

On a test project with caching enabled :

1.) Run tests and make sure they are green.
2.) Introduce a breaking change (`assertTrue(false)`), and the test will be red.
3.) Revert the change. Make sure tests run again and they are green.
4.) Rerun the test without any change. They do not run again but display are
    reported as Green.

(I verified (4) by observing that I don't see TESTING <target-name> in the
output)

On a clean master (3) would report the test as still failing.
diff --git a/src/com/facebook/buck/cli/TestCommand.java b/src/com/facebook/buck/cli/TestCommand.java
index 75db5a0..0a72362 100644
--- a/src/com/facebook/buck/cli/TestCommand.java
+++ b/src/com/facebook/buck/cli/TestCommand.java
@@ -433,15 +433,21 @@
       }
     };
 
+    TestRuleKeyFileHelper testRuleKeyFileHelper = new TestRuleKeyFileHelper(
+        executionContext.getProjectFilesystem());
     for (TestRule test : tests) {
       List<Step> steps;
 
       // Determine whether the test needs to be executed.
-      boolean isTestRunRequired = isTestRunRequiredForTest(test, executionContext);
+      boolean isTestRunRequired =
+          isTestRunRequiredForTest(test, executionContext, testRuleKeyFileHelper);
       if (isTestRunRequired) {
         getBuckEventBus().post(IndividualTestEvent.started(
             options.getArgumentsFormattedAsBuildTargets()));
-        steps = test.runTests(buildContext, executionContext);
+        ImmutableList.Builder<Step> stepsBuilder = ImmutableList.builder();
+        stepsBuilder.addAll(test.runTests(buildContext, executionContext));
+        stepsBuilder.add(testRuleKeyFileHelper.createRuleKeyInDirStep(test));
+        steps = stepsBuilder.build();
       } else {
         steps = ImmutableList.of();
       }
@@ -505,7 +511,11 @@
   }
 
   @VisibleForTesting
-  static boolean isTestRunRequiredForTest(TestRule test, ExecutionContext executionContext) {
+  static boolean isTestRunRequiredForTest(
+      TestRule test,
+      ExecutionContext executionContext,
+      TestRuleKeyFileHelper testRuleKeyFileHelper)
+      throws IOException {
     boolean isTestRunRequired;
     BuildRuleSuccess.Type successType;
     if (executionContext.isDebugEnabled()) {
@@ -513,9 +523,9 @@
       // hook up a debugger.
       isTestRunRequired = true;
     } else if (((successType = test.getBuildResultType()) != null)
-               && (successType == BuildRuleSuccess.Type.FETCHED_FROM_CACHE
-                      || successType == BuildRuleSuccess.Type.MATCHING_RULE_KEY)
-               && test.hasTestResultFiles(executionContext)) {
+               && successType == BuildRuleSuccess.Type.MATCHING_RULE_KEY
+               && test.hasTestResultFiles(executionContext)
+               && testRuleKeyFileHelper.isRuleKeyInDir(test)) {
       // If this build rule's artifacts (which includes the rule's output and its test result
       // files) are up to date, then no commands are necessary to run the tests. The test result
       // files will be read from the XML files in interpretTestResults().
diff --git a/src/com/facebook/buck/cli/TestRuleKeyFileHelper.java b/src/com/facebook/buck/cli/TestRuleKeyFileHelper.java
new file mode 100644
index 0000000..66889dd
--- /dev/null
+++ b/src/com/facebook/buck/cli/TestRuleKeyFileHelper.java
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2013-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License. You may obtain
+ * a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.facebook.buck.cli;
+
+import com.facebook.buck.rules.RuleKey;
+import com.facebook.buck.rules.TestRule;
+import com.facebook.buck.step.Step;
+import com.facebook.buck.step.fs.WriteFileStep;
+import com.facebook.buck.util.ProjectFilesystem;
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import java.io.IOException;
+import java.nio.file.Path;
+
+/**
+ * Helper class to annotate and validate an output directory of a test with a rule key.
+ */
+public class TestRuleKeyFileHelper {
+
+  public static final String RULE_KEY_FILE = ".rulekey";
+
+  private final ProjectFilesystem projectFilesystem;
+
+  public TestRuleKeyFileHelper(ProjectFilesystem projectFilesystem) {
+    this.projectFilesystem = Preconditions.checkNotNull(projectFilesystem);
+  }
+
+  /**
+   * Creates a file in the test's output directory and writes the rule key file in it.
+   * @return A {@link Step} that writes the rule key for the test to it's output directory
+   */
+  public Step createRuleKeyInDirStep(TestRule testRule) throws IOException {
+    RuleKey ruleKey = Preconditions.checkNotNull(testRule.getRuleKey());
+    Path outputDir = Preconditions.checkNotNull(testRule.getPathToTestOutputDirectory());
+    return new WriteFileStep(ruleKey.toString(), getRuleKeyFilePath(outputDir));
+  }
+
+  /**
+   * Checks if a matching rule key file for a test is present in its directoryReturns
+   * @return true if a rule key is written in the specified directory.
+   */
+  public boolean isRuleKeyInDir(TestRule testRule) throws IOException {
+    RuleKey ruleKey = Preconditions.checkNotNull(testRule.getRuleKey());
+    Path outputDir = Preconditions.checkNotNull(testRule.getPathToTestOutputDirectory());
+    Optional<String> ruleKeyOnDisk = projectFilesystem.readFirstLine(getRuleKeyFilePath(outputDir));
+    return ruleKeyOnDisk.isPresent() && ruleKeyOnDisk.get().equals(ruleKey.toString());
+  }
+
+  /**
+   * Get the path file where the rule key is written, given the path to the output directory.
+   */
+  private Path getRuleKeyFilePath(Path pathToOutputDir) {
+    return pathToOutputDir.resolve(RULE_KEY_FILE);
+  }
+}
diff --git a/src/com/facebook/buck/java/JavaTestRule.java b/src/com/facebook/buck/java/JavaTestRule.java
index 93fa6bd..79305d7 100644
--- a/src/com/facebook/buck/java/JavaTestRule.java
+++ b/src/com/facebook/buck/java/JavaTestRule.java
@@ -53,6 +53,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -157,7 +159,7 @@
 
     ImmutableList.Builder<Step> steps = ImmutableList.builder();
 
-    String pathToTestOutput = getPathToTestOutput();
+    Path pathToTestOutput = getPathToTestOutputDirectory();
     MakeCleanDirectoryStep mkdirClean = new MakeCleanDirectoryStep(pathToTestOutput);
     steps.add(mkdirClean);
 
@@ -181,7 +183,7 @@
         classpathEntries,
         testClassNames,
         amendVmArgs(vmArgs, executionContext.getTargetDeviceOptional()),
-        pathToTestOutput,
+        pathToTestOutput.toString(),
         executionContext.isCodeCoverageEnabled(),
         executionContext.isDebugEnabled());
     steps.add(junit);
@@ -229,7 +231,8 @@
       return true;
     }
 
-    File outputDirectory = new File(getPathToTestOutput());
+    File outputDirectory = executionContext.getProjectFilesystem().getFileForRelativePath(
+        getPathToTestOutputDirectory());
     for (String testClass : testClassNames) {
       File testResultFile = new File(outputDirectory, testClass + ".xml");
       if (!testResultFile.isFile()) {
@@ -240,11 +243,13 @@
     return true;
   }
 
-  private String getPathToTestOutput() {
-    return String.format("%s/%s__java_test_%s_output__",
+  @Override
+  public Path getPathToTestOutputDirectory() {
+    return Paths.get(
         BuckConstant.GEN_DIR,
-        getBuildTarget().getBasePathWithSlash(),
-        getBuildTarget().getShortName());
+        getBuildTarget().getBaseNameWithSlash(),
+        String.format("__java_test_%s_output__", getBuildTarget().getShortName())
+    );
   }
 
   @Override
@@ -265,7 +270,7 @@
         ProjectFilesystem filesystem = context.getProjectFilesystem();
         for (String testClass : testClassNames) {
           File testResultFile = filesystem.getFileForRelativePath(
-              String.format("%s/%s.xml", getPathToTestOutput(), testClass));
+              getPathToTestOutputDirectory().resolve(String.format("%s.xml", testClass)));
           TestCaseSummary summary = XmlTestResultParser.parse(testResultFile);
           summaries.add(summary);
         }
diff --git a/src/com/facebook/buck/rules/TestRule.java b/src/com/facebook/buck/rules/TestRule.java
index 4dbdefc..5942b46 100644
--- a/src/com/facebook/buck/rules/TestRule.java
+++ b/src/com/facebook/buck/rules/TestRule.java
@@ -21,6 +21,7 @@
 import com.facebook.buck.test.TestResults;
 import com.google.common.collect.ImmutableSet;
 
+import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.Callable;
 
@@ -62,4 +63,9 @@
    * @return The set of email addresses to act as contact points for this test.
    */
   public ImmutableSet<String> getContacts();
+
+  /**
+   * @return The relative path to the output directory of the test rule.
+   */
+  public Path getPathToTestOutputDirectory();
 }
diff --git a/src/com/facebook/buck/shell/ShTestRule.java b/src/com/facebook/buck/shell/ShTestRule.java
index 1f8acae..9fe731e 100644
--- a/src/com/facebook/buck/shell/ShTestRule.java
+++ b/src/com/facebook/buck/shell/ShTestRule.java
@@ -43,6 +43,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -116,11 +118,13 @@
     return ImmutableList.of(mkdirClean, runTest);
   }
 
-  private String getPathToTestOutputDirectory() {
-    return String.format("%s/%s/__sh_test_%s_output__",
+  @Override
+  public Path getPathToTestOutputDirectory() {
+    return Paths.get(
         BuckConstant.GEN_DIR,
         getBuildTarget().getBasePath(),
-        getBuildTarget().getShortName());
+        String.format("__java_test_%s_output__", getBuildTarget().getShortName())
+    );
   }
 
   @VisibleForTesting
diff --git a/src/com/facebook/buck/step/fs/WriteFileStep.java b/src/com/facebook/buck/step/fs/WriteFileStep.java
index 012428b..7a7b9f6 100644
--- a/src/com/facebook/buck/step/fs/WriteFileStep.java
+++ b/src/com/facebook/buck/step/fs/WriteFileStep.java
@@ -25,17 +25,27 @@
 import com.google.common.io.Files;
 
 import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 
 public class WriteFileStep implements Step {
 
   private final Supplier<String> content;
-  private final String outputPath;
+  private final Path outputPath;
 
   public WriteFileStep(String content, String outputPath) {
+    this(Suppliers.ofInstance(content), Paths.get(outputPath));
+  }
+
+  public WriteFileStep(String content, Path outputPath) {
     this(Suppliers.ofInstance(content), outputPath);
   }
 
   public WriteFileStep(Supplier<String> content, String outputPath) {
+    this(content, Paths.get(outputPath));
+  }
+
+  public WriteFileStep(Supplier<String> content, Path outputPath) {
     this.content = Preconditions.checkNotNull(content);
     this.outputPath = Preconditions.checkNotNull(outputPath);
   }
diff --git a/src/com/facebook/buck/util/ProjectFilesystem.java b/src/com/facebook/buck/util/ProjectFilesystem.java
index a6ca06d..75dd8a8 100644
--- a/src/com/facebook/buck/util/ProjectFilesystem.java
+++ b/src/com/facebook/buck/util/ProjectFilesystem.java
@@ -227,6 +227,15 @@
    * returned. Otherwise, an {@link Optional} with the first line of the file will be returned.
    */
   public Optional<String> readFirstLine(String pathRelativeToProjectRoot) {
+    return readFirstLine(java.nio.file.Paths.get(pathRelativeToProjectRoot));
+  }
+
+  /**
+   * Attempts to read the first line of the file specified by the relative path. If the file does
+   * not exist, is empty, or encounters an error while being read, {@link Optional#absent()} is
+   * returned. Otherwise, an {@link Optional} with the first line of the file will be returned.
+   */
+  public Optional<String> readFirstLine(Path pathRelativeToProjectRoot) {
     Preconditions.checkNotNull(pathRelativeToProjectRoot);
     File file = getFileForRelativePath(pathRelativeToProjectRoot);
     return readFirstLineFromFile(file);
diff --git a/test/com/facebook/buck/cli/TestCommandTest.java b/test/com/facebook/buck/cli/TestCommandTest.java
index caba2af..e50a7ae 100644
--- a/test/com/facebook/buck/cli/TestCommandTest.java
+++ b/test/com/facebook/buck/cli/TestCommandTest.java
@@ -18,6 +18,7 @@
 
 import static com.facebook.buck.util.BuckConstant.GEN_DIR;
 import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
@@ -65,6 +66,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.IOException;
 import java.io.StringWriter;
 import java.util.List;
 import java.util.Map;
@@ -447,38 +449,47 @@
   }
 
   @Test
-  public void testIsTestRunRequiredForTestInDebugMode() {
+  public void testIsTestRunRequiredForTestInDebugMode() throws IOException {
     ExecutionContext executionContext = createMock(ExecutionContext.class);
     expect(executionContext.isDebugEnabled()).andReturn(true);
+
     replay(executionContext);
 
-    assertTrue(TestCommand.isTestRunRequiredForTest(
-        createMock(TestRule.class),
-        executionContext));
+    assertTrue(
+        "In debug mode, test should always run regardless of any cached results since " +
+            "the user is expecting to hook up a debugger.",
+        TestCommand.isTestRunRequiredForTest(
+            createMock(TestRule.class),
+            executionContext,
+            createMock(TestRuleKeyFileHelper.class))
+    );
 
     verify(executionContext);
   }
 
   @Test
-  public void testIsTestRunRequiredForTestBuiltFromCacheIfHasTestResultFiles() {
+  public void testIsTestRunRequiredForTestBuiltFromCacheIfHasTestResultFiles() throws IOException {
     ExecutionContext executionContext = createMock(ExecutionContext.class);
     expect(executionContext.isDebugEnabled()).andReturn(false);
 
     TestRule testRule = createMock(TestRule.class);
     expect(testRule.getBuildResultType()).andReturn(BuildRuleSuccess.Type.FETCHED_FROM_CACHE);
-    expect(testRule.hasTestResultFiles(executionContext)).andReturn(true);
 
     replay(executionContext, testRule);
 
-    assertFalse(TestCommand.isTestRunRequiredForTest(
-        testRule,
-        executionContext));
+    assertTrue(
+        "A cache hit updates the build artifact but not the test results. " +
+            "Therefore, the test should be re-run to ensure the test results are up to date.",
+        TestCommand.isTestRunRequiredForTest(
+            testRule,
+            executionContext,
+            createMock(TestRuleKeyFileHelper.class)));
 
     verify(executionContext, testRule);
   }
 
   @Test
-  public void testIsTestRunRequiredForTestBuiltLocally() {
+  public void testIsTestRunRequiredForTestBuiltLocally() throws IOException {
     ExecutionContext executionContext = createMock(ExecutionContext.class);
     expect(executionContext.isDebugEnabled()).andReturn(false);
 
@@ -487,14 +498,39 @@
 
     replay(executionContext, testRule);
 
-    assertTrue(TestCommand.isTestRunRequiredForTest(
-        testRule,
-        executionContext));
+    assertTrue(
+        "A test built locally should always run regardless of any cached result. ",
+        TestCommand.isTestRunRequiredForTest(
+            testRule,
+            executionContext,
+            createMock(TestRuleKeyFileHelper.class)));
 
     verify(executionContext, testRule);
   }
 
   @Test
+  public void testIsTestRunRequiredIfRuleKeyNotPresent() throws IOException {
+    ExecutionContext executionContext = createMock(ExecutionContext.class);
+    expect(executionContext.isDebugEnabled()).andReturn(false);
+
+    TestRule testRule = createNiceMock(TestRule.class);
+    expect(testRule.getBuildResultType()).andReturn(BuildRuleSuccess.Type.MATCHING_RULE_KEY);
+    expect(testRule.hasTestResultFiles(executionContext)).andReturn(true);
+
+    TestRuleKeyFileHelper testRuleKeyFileHelper = createNiceMock(TestRuleKeyFileHelper.class);
+    expect(testRuleKeyFileHelper.isRuleKeyInDir(testRule)).andReturn(false);
+
+    replay(executionContext, testRule, testRuleKeyFileHelper);
+
+    assertTrue(
+        "A cached build should run the tests if the test output directory\'s rule key is not " +
+            "present or does not matche the rule key for the test.",
+        TestCommand.isTestRunRequiredForTest(testRule, executionContext, testRuleKeyFileHelper));
+
+    verify(executionContext, testRule, testRuleKeyFileHelper);
+  }
+
+  @Test
   public void testIfALabelIsIncludedItShouldNotBeExcluded() throws CmdLineException {
     TestCommandOptions options = new TestCommandOptions(new FakeBuckConfig());
 
diff --git a/test/com/facebook/buck/java/CachedTestIntegrationTest.java b/test/com/facebook/buck/java/CachedTestIntegrationTest.java
index 52fed16..d24b684 100644
--- a/test/com/facebook/buck/java/CachedTestIntegrationTest.java
+++ b/test/com/facebook/buck/java/CachedTestIntegrationTest.java
@@ -55,21 +55,17 @@
    */
   @Test
   public void testPullingJarFromCacheDoesNotResultInReportingStaleTestResults() throws IOException {
-    final Charset charsetForTest = Charsets.UTF_8;
     ProjectWorkspace workspace = TestDataHelper.createProjectWorkspaceForScenario(
         this, "cached_test", tmp);
     workspace.setUp();
+    CachedTestUtil testUtil = new CachedTestUtil(workspace);
 
     // The test should pass out of the box.
     ProcessResult result = workspace.runBuckCommand("test", "//:test");
     result.assertExitCode(0);
 
     // Edit the test so it should fail and then make sure that it fails.
-    File testFile = workspace.getFile("LameTest.java");
-    String originalJavaCode = Files.toString(testFile, charsetForTest);
-    String failingJavaCode = originalJavaCode.replace("String str = \"I am not null.\";",
-        "String str = null;");
-    Files.write(failingJavaCode, testFile, charsetForTest);
+    testUtil.makeTestFail();
     ProcessResult result2 = workspace.runBuckCommand("test", "//:test");
     result2.assertExitCode(1);
     assertThat("`buck test` should fail because testBasicAssertion() failed.",
@@ -77,12 +73,12 @@
         containsString("FAILURE testBasicAssertion"));
 
     // Restore the file to its previous state.
-    Files.write(originalJavaCode, testFile, charsetForTest);
+    testUtil.makeTestSucceed();
     ProcessResult result3 = workspace.runBuckCommand("test", "//:test");
     result3.assertExitCode(0);
 
     // Put the file back in the broken state and make sure the test fails.
-    Files.write(failingJavaCode, testFile, charsetForTest);
+    testUtil.makeTestFail();
     ProcessResult result4 = workspace.runBuckCommand("test", "//:test");
     result4.assertExitCode(1);
     assertThat("`buck test` should fail because testBasicAssertion() failed.",
@@ -90,4 +86,71 @@
         containsString("FAILURE testBasicAssertion"));
   }
 
+  /**
+   * This is similar to {@link #testPullingJarFromCacheDoesNotResultInReportingStaleTestResults()}
+   * but this first builds the test and then runs it. It catches the corner case where the test run
+   * may have the jar read from disk, but the test results are still stale.
+   */
+  @Test
+  public void testRunningTestAfterBuildingWithCacheHitDoesNotReportStaleTests() throws IOException {
+    ProjectWorkspace workspace = TestDataHelper.createProjectWorkspaceForScenario(
+        this, "cached_test", tmp);
+    workspace.setUp();
+    CachedTestUtil testUtil = new CachedTestUtil(workspace);
+
+    // The test should pass out of the box.
+    ProcessResult result = workspace.runBuckCommand("test", "//:test");
+    result.assertExitCode(0);
+
+    // Edit the test so it should fail and then make sure that it fails.
+    testUtil.makeTestFail();
+    workspace.runBuckCommand("build", "//:test")
+        .assertExitCode("The test should build successfully, but will fail when executed.", 0);
+    ProcessResult result2 = workspace.runBuckCommand("test", "//:test");
+    result2.assertExitCode(1);
+    assertThat("`buck test` should fail because testBasicAssertion() failed.",
+        result2.getStderr(),
+        containsString("FAILURE testBasicAssertion"));
+
+    // Restore the file to its previous state.
+    testUtil.makeTestSucceed();
+    workspace.runBuckCommand("build", "//:test")
+        .assertExitCode("The test should build successfully.", 0);
+    ProcessResult result3 = workspace.runBuckCommand("test", "//:test");
+    result3.assertExitCode(0);
+
+    // Put the file back in the broken state and make sure the test fails.
+    testUtil.makeTestFail();
+    workspace.runBuckCommand("build", "//:test")
+        .assertExitCode("The test should build successfully, but will fail when executed.", 0);
+    ProcessResult result4 = workspace.runBuckCommand("test", "//:test");
+    result4.assertExitCode(1);
+    assertThat("`buck test` should fail because testBasicAssertion() failed.",
+        result4.getStderr(),
+        containsString("FAILURE testBasicAssertion"));
+  }
+
+  private static class CachedTestUtil {
+
+    private static final String TEST_FILE = "LameTest.java";
+    private static final Charset CHARSET = Charsets.UTF_8;
+    private final String originalJavaCode;
+    private final File testFile;
+    private final String failingJavaCode;
+
+    CachedTestUtil(ProjectWorkspace workspace) throws IOException {
+      this.testFile = workspace.getFile(TEST_FILE);
+      this.originalJavaCode = Files.toString(testFile, CHARSET);
+      this.failingJavaCode = originalJavaCode.replace("String str = \"I am not null.\";",
+          "String str = null;");
+    }
+
+    public void makeTestFail() throws IOException {
+      Files.write(failingJavaCode, testFile, CHARSET);
+    }
+
+    public void makeTestSucceed() throws IOException {
+      Files.write(originalJavaCode, testFile, CHARSET);
+    }
+  }
 }
diff --git a/test/com/facebook/buck/rules/FakeTestRule.java b/test/com/facebook/buck/rules/FakeTestRule.java
index 79fd7ab..f88bca3 100644
--- a/test/com/facebook/buck/rules/FakeTestRule.java
+++ b/test/com/facebook/buck/rules/FakeTestRule.java
@@ -25,6 +25,7 @@
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.util.concurrent.ListenableFuture;
 
+import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.Callable;
 
@@ -100,4 +101,9 @@
   public ImmutableSet<String> getContacts() {
     return ImmutableSet.of();
   }
+
+  @Override
+  public Path getPathToTestOutputDirectory() {
+    throw new UnsupportedOperationException("getPathToTestOutput() not supported in fake");
+  }
 }
diff --git a/test/com/facebook/buck/util/ProjectFilesystemTest.java b/test/com/facebook/buck/util/ProjectFilesystemTest.java
index a5d6122..9bc5add 100644
--- a/test/com/facebook/buck/util/ProjectFilesystemTest.java
+++ b/test/com/facebook/buck/util/ProjectFilesystemTest.java
@@ -31,6 +31,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
 
 /** Unit test for {@link ProjectFilesystem}. */
 public class ProjectFilesystemTest {
@@ -61,8 +62,13 @@
   }
 
   @Test(expected = NullPointerException.class)
+  public void testReadFirstLineRejectsNullString() {
+    filesystem.readFirstLine(/* pathRelativeToProjectRoot */ (String) null);
+  }
+
+  @Test(expected = NullPointerException.class)
   public void testReadFirstLineRejectsNullPath() {
-    filesystem.readFirstLine(/* pathRelativeToProjectRoot */ null);
+    filesystem.readFirstLine(/* pathRelativeToProjectRoot */ (Path) null);
   }
 
   @Test