From fe7deabfa094e35a63b83e7912efeb8097c71bc8 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 5 Aug 2022 15:03:21 -0700 Subject: [PATCH] Restrict visibility of all members of `CommandLines.ParamFileActionInput` to private. Also clean up `CommandLinesTest` to use existing public getters for `CommandLines.ParamFileActionInput` and expand the checks on argument lists to validate the order. PiperOrigin-RevId: 465655870 Change-Id: I76eb3fbd5d50152e3b16eeb791af5024aa2846e4 --- .../build/lib/actions/CommandLines.java | 8 ++-- .../build/lib/actions/CommandLinesTest.java | 38 +++++++++++-------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java index 85bcf741a7a356..81efe6ec9a2a31 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java @@ -276,10 +276,10 @@ public List getParamFiles() { /** An in-memory param file virtual action input. */ public static final class ParamFileActionInput implements VirtualActionInput { - final PathFragment paramFileExecPath; - final Iterable arguments; - final ParameterFileType type; - final Charset charset; + private final PathFragment paramFileExecPath; + private final Iterable arguments; + private final ParameterFileType type; + private final Charset charset; public ParamFileActionInput( PathFragment paramFileExecPath, diff --git a/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java b/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java index c766287ecd8b93..8a57a3df36d000 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/CommandLinesTest.java @@ -44,8 +44,8 @@ public void expand_simpleCommandLine_returnsCorrectCommandLine() throws Exceptio ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, CommandAdjuster.NOOP, 0); - assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar"); - assertThat(expanded.arguments()).containsExactly("--foo", "--bar"); + assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar").inOrder(); + assertThat(expanded.arguments()).containsExactly("--foo", "--bar").inOrder(); assertThat(expanded.getParamFiles()).isEmpty(); } @@ -56,8 +56,8 @@ public void expand_commandLineFromArguments_returnsCorrectCommandLine() throws E ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, CommandAdjuster.NOOP, 0); - assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar"); - assertThat(expanded.arguments()).containsExactly("--foo", "--bar"); + assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar").inOrder(); + assertThat(expanded.arguments()).containsExactly("--foo", "--bar").inOrder(); assertThat(expanded.getParamFiles()).isEmpty(); } @@ -88,10 +88,12 @@ public void expand_paramFileUseAlways_returnsCommandLineWithParamFile() throws E ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, CommandAdjuster.NOOP, 0); - assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar"); + assertThat(commandLines.allArguments()).containsExactly("--foo", "--bar").inOrder(); assertThat(expanded.arguments()).containsExactly("@output.txt-0.params"); assertThat(expanded.getParamFiles()).hasSize(1); - assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("--foo", "--bar"); + assertThat(expanded.getParamFiles().get(0).getArguments()) + .containsExactly("--foo", "--bar") + .inOrder(); } @Test @@ -107,7 +109,7 @@ public void expand_paramFileCommandWithinLimits_returnsNoParamFile() throws Exce ExpandedCommandLines expanded = commandLines.expand(artifactExpander, execPath, NO_LIMIT, CommandAdjuster.NOOP, 0); - assertThat(expanded.arguments()).containsExactly("--foo", "--bar"); + assertThat(expanded.arguments()).containsExactly("--foo", "--bar").inOrder(); assertThat(expanded.getParamFiles()).isEmpty(); } @@ -127,7 +129,9 @@ public void expand_paramFileCommandOverLimits_returnsParamFile() throws Exceptio assertThat(expanded.arguments()).containsExactly("@output.txt-0.params"); assertThat(expanded.getParamFiles()).hasSize(1); - assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("--foo", "--bar"); + assertThat(expanded.getParamFiles().get(0).getArguments()) + .containsExactly("--foo", "--bar") + .inOrder(); } @Test @@ -151,11 +155,11 @@ public void expand_mixOfCommandLinesAndParamFiles_returnsCorrectCommandLines() t assertThat(expanded.arguments()) .containsExactly("a", "b", "@output.txt-0.params", "e", "f", "@output.txt-1.params"); assertThat(expanded.getParamFiles()).hasSize(2); - assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("c", "d"); - assertThat(expanded.getParamFiles().get(0).paramFileExecPath.getPathString()) + assertThat(expanded.getParamFiles().get(0).getArguments()).containsExactly("c", "d").inOrder(); + assertThat(expanded.getParamFiles().get(0).getExecPathString()) .isEqualTo("output.txt-0.params"); - assertThat(expanded.getParamFiles().get(1).arguments).containsExactly("g", "h"); - assertThat(expanded.getParamFiles().get(1).paramFileExecPath.getPathString()) + assertThat(expanded.getParamFiles().get(1).getArguments()).containsExactly("g", "h").inOrder(); + assertThat(expanded.getParamFiles().get(1).getExecPathString()) .isEqualTo("output.txt-1.params"); } @@ -176,10 +180,10 @@ public void expand_commandsWithParamFilesSecondExceedsLimits_returnsParamFileFor commandLines.expand( artifactExpander, execPath, new CommandLineLimits(4), CommandAdjuster.NOOP, 0); - assertThat(commandLines.allArguments()).containsExactly("a", "b", "c", "d"); - assertThat(expanded.arguments()).containsExactly("a", "b", "@output.txt-0.params"); + assertThat(commandLines.allArguments()).containsExactly("a", "b", "c", "d").inOrder(); + assertThat(expanded.arguments()).containsExactly("a", "b", "@output.txt-0.params").inOrder(); assertThat(expanded.getParamFiles()).hasSize(1); - assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("c", "d"); + assertThat(expanded.getParamFiles().get(0).getArguments()).containsExactly("c", "d").inOrder(); } /** Filtering of flag and positional arguments with flagsOnly. */ @@ -201,6 +205,8 @@ public void expand_flagsOnly_movesOnlyDashDashPrefixedFlagsToParamFile() throws assertThat(commandLines.allArguments()).containsExactly("--a", "1", "--b=c", "-2"); assertThat(expanded.arguments()).containsExactly("1", "-2", "@output.txt-0.params"); assertThat(expanded.getParamFiles()).hasSize(1); - assertThat(expanded.getParamFiles().get(0).arguments).containsExactly("--a", "--b=c"); + assertThat(expanded.getParamFiles().get(0).getArguments()) + .containsExactly("--a", "--b=c") + .inOrder(); } }