CSharpLibrary fixes

Summary: The previous version of test runs only when `csc` is available on PATH. It is not the case by default. Test was updates to run with VS 2015 by default (see https://msdn.microsoft.com/en-us/library/1700bbwd.aspx). References should not be escaped since the whole argument is escaped (It seems that automatic escaping of arguments was introduced a long time ago, and we did not run CsharpLibraryIntegrationTest). Test Plan: CI Reviewed By: marcinkosiba fbshipit-source-id: db89058
classicvalues · Apr 26, 2017 · 24b4b2a · 24b4b2a
1 parent 6f96e63
commit 24b4b2a
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 33 deletions.
diff --git a/src/com/facebook/buck/dotnet/CsharpLibraryCompile.java b/src/com/facebook/buck/dotnet/CsharpLibraryCompile.java
@@ -84,11 +84,11 @@ protected ImmutableList<String> getShellCommandInternal(ExecutionContext context
 
   private String resolveReference(DotnetFramework netFramework, Either<Path, String> ref) {
     if (ref.isLeft()) {
-      return Escaper.escapeAsShellString(ref.getLeft().toString());
+      return ref.getLeft().toString();
     }
 
     Path pathToAssembly = netFramework.findReferenceAssembly(ref.getRight());
-    return Escaper.escapeAsShellString(pathToAssembly.toString());
+    return pathToAssembly.toString();
   }
 
   @Override

diff --git a/test/com/facebook/buck/dotnet/BUCK b/test/com/facebook/buck/dotnet/BUCK
@@ -140,6 +140,7 @@ standard_java_test(
         "//src/com/facebook/buck/zip:constants",
         "//src/com/facebook/buck/zip:stream",
         "//test/com/facebook/buck/dotnet:testutil",
+        "//test/com/facebook/buck/testutil:testutil",
         "//test/com/facebook/buck/testutil/integration:util",
         "//third-party/java/aether:aether-api",
         "//third-party/java/android:tools-sdk-common",

diff --git a/test/com/facebook/buck/dotnet/CsharpLibraryIntegrationTest.java b/test/com/facebook/buck/dotnet/CsharpLibraryIntegrationTest.java
@@ -17,74 +17,82 @@
 package com.facebook.buck.dotnet;
 
 import static com.facebook.buck.dotnet.DotnetAssumptions.assumeCscIsAvailable;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import com.facebook.buck.testutil.TestConsole;
 import com.facebook.buck.testutil.integration.ProjectWorkspace;
 import com.facebook.buck.testutil.integration.TemporaryPaths;
 import com.facebook.buck.testutil.integration.TestDataHelper;
+import com.facebook.buck.util.DefaultProcessExecutor;
+import com.facebook.buck.util.ProcessExecutor;
+import com.facebook.buck.util.ProcessExecutorParams;
+import com.facebook.buck.util.environment.Platform;
+import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Optional;
+import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
 public class CsharpLibraryIntegrationTest {
 
+  // https://msdn.microsoft.com/en-us/library/1700bbwd.aspx
+  private static final String vsvars32bat =
+      "C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\Common7\\Tools\\vsvars32.bat";
+
   @Rule public TemporaryPaths tmp = new TemporaryPaths();
 
+  private ImmutableMap<String, String> env;
+
+  @Before
+  public void setUp() throws IOException, InterruptedException {
+    env = getEnv();
+    assumeCscIsAvailable(env);
+  }
+
   @Test
   public void shouldCompileLibraryWithSystemProvidedDeps() throws IOException {
-    assumeCscIsAvailable();
-
     ProjectWorkspace workspace =
         TestDataHelper.createProjectWorkspaceForScenario(this, "csc-tests", tmp);
     workspace.setUp();
 
-    Path output = workspace.buildAndReturnOutput("//src:simple");
-
-    assertTrue(output.getFileName().toString().endsWith(".dll"));
-    assertTrue("File doesn't seem to have been compiled", Files.size(output) > 0);
+    ProjectWorkspace.ProcessResult result = workspace.runBuckCommand(env, "build", "//src:simple");
+    result.assertSuccess();
   }
 
   @Test
   public void shouldCompileLibraryWithAPrebuiltDependency() throws IOException {
-    assumeCscIsAvailable();
-
     ProjectWorkspace workspace =
         TestDataHelper.createProjectWorkspaceForScenario(this, "csc-tests", tmp);
     workspace.setUp();
 
-    Path output = workspace.buildAndReturnOutput("//src:prebuilt");
-
-    assertTrue(output.getFileName().toString().endsWith(".dll"));
+    ProjectWorkspace.ProcessResult result =
+        workspace.runBuckCommand(env, "build", "//src:prebuilt");
+    result.assertSuccess();
   }
 
   @Test
   public void shouldBeAbleToEmbedResourcesIntoTheBuiltDll() throws IOException {
-    assumeCscIsAvailable();
-
     ProjectWorkspace workspace =
         TestDataHelper.createProjectWorkspaceForScenario(this, "csc-tests", tmp);
     workspace.setUp();
 
-    Path output = workspace.buildAndReturnOutput("//src:embed");
-
-    assertTrue(output.getFileName().toString().endsWith(".dll"));
-    assertTrue("File doesn't seem to have been compiled", Files.size(output) > 0);
+    ProjectWorkspace.ProcessResult result = workspace.runBuckCommand(env, "build", "//src:embed");
+    result.assertSuccess();
   }
 
   @Test
   public void shouldBeAbleToDependOnAnotherCsharpLibrary() throws IOException {
-    assumeCscIsAvailable();
-
     ProjectWorkspace workspace =
         TestDataHelper.createProjectWorkspaceForScenario(this, "csc-tests", tmp);
     workspace.setUp();
 
-    ProjectWorkspace.ProcessResult result = workspace.runBuckBuild("//src:dependent");
-
+    ProjectWorkspace.ProcessResult result =
+        workspace.runBuckCommand(env, "build", "//src:dependent");
     result.assertSuccess();
   }
 
@@ -93,4 +101,31 @@ public void shouldBeAbleToDependOnAnotherCsharpLibrary() throws IOException {
   public void shouldBeAbleToAddTheSameResourceToADllTwice() {
     fail("Implement me, please!");
   }
+
+  private ImmutableMap<String, String> getEnv() throws IOException, InterruptedException {
+    if (Platform.detect() == Platform.WINDOWS && Files.exists(Paths.get(vsvars32bat))) {
+      String vsvar32BatEsc = vsvars32bat.replace(" ", "^ ").replace("(", "^(");
+      ProcessExecutorParams params =
+          ProcessExecutorParams.ofCommand("cmd", "/c", vsvar32BatEsc + " && set");
+      ProcessExecutor executor = new DefaultProcessExecutor(new TestConsole());
+      ProcessExecutor.Result envResult = executor.launchAndExecute(params);
+      Optional<String> envOut = envResult.getStdout();
+      Assert.assertTrue(envOut.isPresent());
+      String envString = envOut.get();
+      String[] envStrings = envString.split("\\r?\\n");
+      ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+      for (String s : envStrings) {
+        int sep = s.indexOf('=');
+        String key = s.substring(0, sep);
+        if ("PATH".equalsIgnoreCase(key)) {
+          key = "PATH";
+        }
+        String val = s.substring(sep + 1, s.length());
+        builder.put(key, val);
+      }
+      return builder.build();
+    } else {
+      return ImmutableMap.copyOf(System.getenv());
+    }
+  }
 }
diff --git a/test/com/facebook/buck/dotnet/DotnetAssumptions.java b/test/com/facebook/buck/dotnet/DotnetAssumptions.java
@@ -30,10 +30,8 @@ private DotnetAssumptions() {
     // Utility class
   }
 
-  public static void assumeCscIsAvailable() {
-    Optional<Path> csc =
-        new ExecutableFinder()
-            .getOptionalExecutable(Paths.get("csc"), ImmutableMap.copyOf(System.getenv()));
+  public static void assumeCscIsAvailable(ImmutableMap<String, String> env) {
+    Optional<Path> csc = new ExecutableFinder().getOptionalExecutable(Paths.get("csc"), env);
 
     assumeTrue("Unable to find csc", csc.isPresent());
   }

diff --git a/windows_failures.txt b/windows_failures.txt
@@ -28,10 +28,6 @@
 !com.facebook.buck.cxx.HeaderMapStepTest
 !com.facebook.buck.cxx.OmnibusTest
 !com.facebook.buck.cxx.ToolTest
-!com.facebook.buck.dotnet.CsharpLibraryIntegrationTest
-!com.facebook.buck.dotnet.CsharpLibraryIntegrationTest#shouldBeAbleToEmbedResourcesIntoTheBuiltDll
-!com.facebook.buck.dotnet.CsharpLibraryIntegrationTest#shouldCompileLibraryWithAPrebuiltDependency
-!com.facebook.buck.dotnet.CsharpLibraryIntegrationTest#shouldCompileLibraryWithSystemProvidedDeps
 !com.facebook.buck.python.PrebuiltPythonLibraryIntegrationTest
 !com.facebook.buck.python.PythonBinaryIntegrationTest
 !com.facebook.buck.python.PythonSrcZipIntegrationTest#testDependingOnSrcZipWorks