Skip to content

Commit

Permalink
Replace --cpu and --host_cpu with --platforms and `--host_platf…
Browse files Browse the repository at this point in the history
…orm` respectively in `BuildViewTestCase`

Modify failing tests to use platform name in artifacts paths instead of CPU name.

PiperOrigin-RevId: 628306990
Change-Id: I921b8aff6551ea77f809b9a408d0cd67c771536a
  • Loading branch information
mai93 authored and copybara-github committed Apr 26, 2024
1 parent 3beaaaf commit f8273c2
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,10 @@ protected BuildOptions createBuildOptions(String... args)
// TODO(dmarting): Add --stamp option only to test that requires it.
allArgs.add("--stamp"); // Stamp is now defaulted to false.
allArgs.add("--experimental_extended_sanity_checks");
// Always default to k8, even on mac and windows. Tests that need different cpu should set it
// using {@link useConfiguration()} explicitly.
allArgs.add("--cpu=k8");
allArgs.add("--host_cpu=k8");
// Always default to k8, even on mac and windows. Tests that need different platform should set
// it using {@link useConfiguration()} with (--platforms=foo) explicitly.
allArgs.add("--platforms=" + TestConstants.PLATFORM_LABEL);
allArgs.add("--host_platform=" + TestConstants.PLATFORM_LABEL);

// Now the flags from the test.
allArgs.add(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,18 @@ public void testScriptFileIsUsedForBatchCmd() throws Exception {
"outs = ['message.txt'],",
"cmd_bat = ' && '.join([\"echo \\\"Hello, Batch cmd, %s.\\\" >$(location message.txt)\" %"
+ " i for i in range(1, 1000)]),)");
useConfiguration(
"--platforms=//platforms:windows",
"--host_platform=//platforms:windows",
"--experimental_platform_in_output_dir");

Artifact messageArtifact = getFileConfiguredTarget("//genrule1:message.txt").getArtifact();
SpawnAction shellAction = (SpawnAction) getGeneratingAction(messageArtifact);

assertThat(shellAction).isNotNull();
assertThat(shellAction.getOutputs()).containsExactly(messageArtifact);

String expected = "bazel-out\\k8-fastbuild\\bin\\genrule1\\hello_world.genrule_script.bat";
String expected = "bazel-out\\windows-fastbuild\\bin\\genrule1\\hello_world.genrule_script.bat";
assertThat(shellAction.getArguments().get(0)).isEqualTo("cmd.exe");
int last = shellAction.getArguments().size() - 1;
assertThat(shellAction.getArguments().get(last - 1)).isEqualTo("/c");
Expand All @@ -156,14 +160,19 @@ public void testScriptFileIsUsedForPowershellCmd() throws Exception {
"outs = ['message.txt'],",
"cmd_ps = '; '.join([\"echo \\\"Hello, Powershell cmd, %s.\\\" >$(location message.txt)\""
+ " % i for i in range(1, 1000)]),)");
useConfiguration(
"--platforms=//platforms:windows",
"--host_platform=//platforms:windows",
"--experimental_platform_in_output_dir");

Artifact messageArtifact = getFileConfiguredTarget("//genrule1:message.txt").getArtifact();
SpawnAction shellAction = (SpawnAction) getGeneratingAction(messageArtifact);

assertThat(shellAction).isNotNull();
assertThat(shellAction.getOutputs()).containsExactly(messageArtifact);

String expected = ".\\bazel-out\\k8-fastbuild\\bin\\genrule1\\hello_world.genrule_script.ps1";
String expected =
".\\bazel-out\\windows-fastbuild\\bin\\genrule1\\hello_world.genrule_script.ps1";
assertThat(shellAction.getArguments().get(0)).isEqualTo("powershell.exe");
assertThat(shellAction.getArguments().get(1)).isEqualTo("/c");
assertPowershellCommandEquals(expected, shellAction.getArguments().get(2));
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ java_test(
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
"//third_party:junit4",
"//third_party:truth",
],
Expand Down Expand Up @@ -392,6 +393,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig;
import com.google.devtools.build.lib.rules.genrule.GenRuleAction;
import com.google.devtools.build.lib.testutil.TestConstants;
import java.util.regex.Pattern;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -40,7 +41,14 @@ public void testActionGraph() throws Exception {
AnalysisMock.get()
.ccSupport()
.setupCcToolchainConfig(mockToolsConfig, CcToolchainConfig.builder());
useConfiguration("--fdo_profile=//:mock_profile", "--compilation_mode=opt");
useConfiguration(
"--fdo_profile=//:mock_profile",
"--compilation_mode=opt",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));

scratch.file("binary.cc", "int main() { return 0; }");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,12 @@ public void testCompilationParameterFile() throws Exception {
mockToolsConfig,
CcToolchainConfig.builder().withFeatures(CppRuleClasses.COMPILER_PARAM_FILE));
scratch.file("a/BUILD", "cc_library(name='foo', srcs=['foo.cc'])");
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
CppCompileAction cppCompileAction = getCppCompileAction("//a:foo");
assertThat(
cppCompileAction.getArguments().stream()
Expand All @@ -1417,6 +1423,12 @@ public void testCppCompileActionArgvIgnoreParamFile() throws Exception {
mockToolsConfig,
CcToolchainConfig.builder().withFeatures(CppRuleClasses.COMPILER_PARAM_FILE));
scratch.file("a/BUILD", "cc_library(name='foo', srcs=['foo.cc'])");
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
CppCompileAction cppCompileAction = getCppCompileAction("//a:foo");
ImmutableList<String> argv =
cppCompileAction.getStarlarkArgv().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,14 @@ public void testFilesToBuild() throws Exception {
.withFeatures(
CppRuleClasses.SUPPORTS_DYNAMIC_LINKER,
CppRuleClasses.SUPPORTS_INTERFACE_SHARED_LIBRARIES));
useConfiguration("--platforms=" + TestConstants.PLATFORM_LABEL);
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
ConfiguredTarget hello = getConfiguredTarget("//hello:hello");
String cpu = getTargetConfiguration().getCpu();
String cpu = "k8"; // CPU of the platform specified with --platforms
Artifact archive = getBinArtifact("libhello.a", hello);
Artifact implSharedObject = getBinArtifact("libhello.so", hello);
Artifact implInterfaceSharedObject = getBinArtifact("libhello.ifso", hello);
Expand Down Expand Up @@ -264,9 +269,14 @@ public void testFilesToBuildWithInterfaceSharedObjects() throws Exception {
.withFeatures(
CppRuleClasses.SUPPORTS_DYNAMIC_LINKER,
CppRuleClasses.SUPPORTS_INTERFACE_SHARED_LIBRARIES));
useConfiguration("--platforms=" + TestConstants.PLATFORM_LABEL);
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
ConfiguredTarget hello = getConfiguredTarget("//hello:hello");
String cpu = getTargetConfiguration().getCpu();
String cpu = "k8"; // CPU of the platform specified with --platforms
Artifact archive = getBinArtifact("libhello.a", hello);
Artifact sharedObject = getBinArtifact("libhello.ifso", hello);
Artifact implSharedObject = getBinArtifact("libhello.so", hello);
Expand Down Expand Up @@ -2201,7 +2211,13 @@ public void testRpathAndLinkPathsWithoutTransitions() throws Exception {
CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));

prepareCustomTransition();
useConfiguration("--platforms=" + TestConstants.PLATFORM_LABEL, "--compilation_mode=fastbuild");
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--compilation_mode=fastbuild",
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));

scratch.file(
"no-transition/BUILD",
Expand Down Expand Up @@ -2253,7 +2269,13 @@ public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception
CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));

prepareCustomTransition();
useConfiguration("--platforms=" + TestConstants.PLATFORM_LABEL, "--compilation_mode=fastbuild");
useConfiguration(
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--compilation_mode=fastbuild",
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));

scratch.file(
"transition/BUILD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig;
import com.google.devtools.build.lib.packages.util.MockPlatformSupport;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -324,7 +325,13 @@ public void testExternalIncludePathsVariable() throws Exception {
.setupCcToolchainConfig(
mockToolsConfig,
CcToolchainConfig.builder().withFeatures(CppRuleClasses.EXTERNAL_INCLUDE_PATHS));
useConfiguration("--features=external_include_paths");
useConfiguration(
"--features=external_include_paths",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
scratch.appendFile("WORKSPACE", "local_repository(", " name = 'pkg',", " path = '/foo')");
getSkyframeExecutor()
.invalidateFilesUnderPathForTesting(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@ public void dynamicLink_siblingLayout_externalBinary_rpath() throws Exception {
useConfiguration(
"--extra_toolchains=//toolchain:toolchain",
"--dynamic_mode=fully",
"--incompatible_enable_cc_toolchain_resolution");
"--incompatible_enable_cc_toolchain_resolution",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));

ConfiguredTarget target = getConfiguredTarget("@src//test:foo");
assertThat(target).isNotNull();
Expand Down Expand Up @@ -264,7 +269,12 @@ public void dynamicLink_siblingLayout_externalToolchain_rpath() throws Exception
useConfiguration(
"--extra_toolchains=@toolchain//:toolchain",
"--dynamic_mode=fully",
"--incompatible_enable_cc_toolchain_resolution");
"--incompatible_enable_cc_toolchain_resolution",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));

ConfiguredTarget target = getConfiguredTarget("//src/test:foo");
assertThat(target).isNotNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6284,35 +6284,65 @@ public void testTransitiveLinkWithCompilationOutputs() throws Exception {

@Test
public void testLinkStampExpliciltyEnabledOverridesNoStampFlag() throws Exception {
useConfiguration("--nostamp");
useConfiguration(
"--nostamp",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
setupTestTransitiveLink(scratch, "stamp=1", "linking_contexts=dep_linking_contexts");
assertStampEnabled(getLinkstampCompileAction("//foo:bin"));
}

@Test
public void testLinkExplicitlyDisabledOverridesStampFlag() throws Exception {
useConfiguration("--nostamp");
useConfiguration(
"--nostamp",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
setupTestTransitiveLink(scratch, "stamp=0", "linking_contexts=dep_linking_contexts");
assertStampDisabled(getLinkstampCompileAction("//foo:bin"));
}

@Test
public void testLinkStampUseFlagStamp() throws Exception {
useConfiguration("--stamp");
useConfiguration(
"--stamp",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
setupTestTransitiveLink(scratch, "stamp=-1", "linking_contexts=dep_linking_contexts");
assertStampEnabled(getLinkstampCompileAction("//foo:bin"));
}

@Test
public void testLinkStampUseFlagNoStamp() throws Exception {
useConfiguration("--nostamp");
useConfiguration(
"--nostamp",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
setupTestTransitiveLink(scratch, "stamp=-1", "linking_contexts=dep_linking_contexts");
assertStampDisabled(getLinkstampCompileAction("//foo:bin"));
}

@Test
public void testLinkStampDisabledByDefaultDespiteStampFlag() throws Exception {
useConfiguration("--stamp");
useConfiguration(
"--stamp",
"--platforms=" + TestConstants.PLATFORM_LABEL,
"--experimental_platform_in_output_dir",
String.format(
"--experimental_override_name_platform_in_output_dir=%s=k8",
TestConstants.PLATFORM_LABEL));
setupTestTransitiveLink(scratch, "linking_contexts=dep_linking_contexts");
assertStampDisabled(getLinkstampCompileAction("//foo:bin"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,11 @@ public void testJ2ObjcProtoClassMappingFilesExportedInJavaLibrary() throws Excep
" srcs = ['test.java'],",
" deps = [':test_java_proto']",
")");
useConfiguration(
"--proto_toolchain_for_java=//tools/proto/toolchains:java",
"--platforms=" + MockObjcSupport.DARWIN_X86_64,
"--apple_platform_type=macos",
"--cpu=darwin_x86_64");

ConfiguredTarget target = getJ2ObjCAspectConfiguredTarget(
"//java/com/google/dummy/test/proto:test");
Expand All @@ -303,7 +308,7 @@ public void testJ2ObjcProtoClassMappingFilesExportedInJavaLibrary() throws Excep

assertThat(classMappingFilesList.get(0).getExecPathString())
.containsMatch(
"/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin/java/com/google/dummy/test/proto/test.clsmap.properties");
"/darwin_x86_64-fastbuild/bin/java/com/google/dummy/test/proto/test.clsmap.properties");
}

@Test
Expand All @@ -324,22 +329,26 @@ public void testJavaProtoLibraryWithProtoLibrary() throws Exception {
" srcs = ['test.java'],",
" deps = [':test_java_proto']",
")");
useConfiguration(
"--proto_toolchain_for_java=//tools/proto/toolchains:java",
"--platforms=" + MockObjcSupport.DARWIN_X86_64,
"--apple_platform_type=macos",
"--cpu=darwin_x86_64");

ConfiguredTarget target = getJ2ObjCAspectConfiguredTarget("//x:test");
StructImpl j2ObjcMappingFileInfo = getJ2ObjcMappingFileInfoFromTarget(target);
ImmutableList<Artifact> classMappingFilesList =
getArtifacts(j2ObjcMappingFileInfo, "class_mapping_files");
assertThat(classMappingFilesList.get(0).getExecPathString())
.containsMatch(
"/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin/x/test.clsmap.properties");
.containsMatch("/darwin_x86_64-fastbuild/bin/x/test.clsmap.properties");

StarlarkInfo objcProvider = getObjcInfo(target);
CcCompilationContext ccCompilationContext =
target.get(CcInfo.PROVIDER).getCcCompilationContext();
assertThat(ccCompilationContext.getDeclaredIncludeSrcs().toList().toString())
.containsMatch("/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin]x/test.j2objc.pb.h");
.containsMatch("/darwin_x86_64-fastbuild/bin]x/test.j2objc.pb.h");
assertThat(getSource(objcProvider).toString())
.containsMatch("/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin]x/test.j2objc.pb.m,");
.containsMatch("/darwin_x86_64-fastbuild/bin]x/test.j2objc.pb.m,");
}

@Test
Expand Down Expand Up @@ -374,6 +383,11 @@ public void testJavaProtoLibraryWithProtoLibrary_external() throws Exception {
deps = ["@bla//foo:test_java_proto"],
)
""");
useConfiguration(
"--proto_toolchain_for_java=//tools/proto/toolchains:java",
"--platforms=" + MockObjcSupport.DARWIN_X86_64,
"--apple_platform_type=macos",
"--cpu=darwin_x86_64");

ConfiguredTarget target = getJ2ObjCAspectConfiguredTarget("//x:test");

Expand All @@ -382,19 +396,16 @@ public void testJavaProtoLibraryWithProtoLibrary_external() throws Exception {
getArtifacts(j2ObjcMappingFileInfo, "class_mapping_files");

assertThat(classMappingFilesList.get(0).getExecPathString())
.containsMatch(
"/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin/external/bla/foo/test.clsmap.properties");
.containsMatch("/darwin_x86_64-fastbuild/bin/external/bla/foo/test.clsmap.properties");

StarlarkInfo objcProvider = getObjcInfo(target);
CcCompilationContext ccCompilationContext =
target.get(CcInfo.PROVIDER).getCcCompilationContext();

assertThat(ccCompilationContext.getDeclaredIncludeSrcs().toList().toString())
.containsMatch(
"/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin]external/bla/foo/test.j2objc.pb.h");
.containsMatch("/darwin_x86_64-fastbuild/bin]external/bla/foo/test.j2objc.pb.h");
assertThat(getSource(objcProvider).toString())
.containsMatch(
"/darwin_x86_64-fastbuild-applebin_macos-ST-[^/]*/bin]external/bla/foo/test.j2objc.pb.m");
.containsMatch("/darwin_x86_64-fastbuild/bin]external/bla/foo/test.j2objc.pb.m");
assertThat(ccCompilationContext.getIncludeDirs())
.contains(
getConfiguration(target)
Expand Down
Loading

0 comments on commit f8273c2

Please sign in to comment.