Skip to content

Commit

Permalink
Allow BuildViewTestCase to ignore some options when comparing configu…
Browse files Browse the repository at this point in the history
…rations

Rather than simply comparing the two configuration, the test author can now include a set of options to 'exclude' from the comparison. They are trimmed away and resultant trimmed configurations are then compared.

Includes a related fix to GoConfiguredTargetTest to ignore TestOptions from the comparison (since the case where trim_test_configuration is on, the actual configuration will not include TestOptions and this is fine).

PiperOrigin-RevId: 366134162
  • Loading branch information
twigg authored and copybara-github committed Mar 31, 2021
1 parent 4970e0f commit b7308f8
Showing 1 changed file with 37 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.util;

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -87,7 +90,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
Expand Down Expand Up @@ -177,7 +180,6 @@
import com.google.devtools.common.options.OptionsParsingException;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -709,17 +711,42 @@ protected final ConfiguredTargetAndData getConfiguredTargetAndDataDirectPrerequi
}

/**
* Asserts that two configurations are the same.
* Returns a BuildOptions with options in exclude trimmed away.
*
* <p>Historically this meant they contained the same object reference. But with upcoming dynamic
* configurations that may no longer be true (for example, they may have the same values but not
* the same {@link Fragment}s. So this method abstracts the "configuration equivalency" checking
* into one place, where the implementation logic can evolve as needed.
* <p>BuildOptions.trim actually retains the options passed to it so must reverse the logic.
*/
protected void assertConfigurationsEqual(BuildConfiguration config1, BuildConfiguration config2) {
private static BuildOptions trimConfiguration(
BuildOptions input, Set<Class<? extends FragmentOptions>> exclude) {
Set<Class<? extends FragmentOptions>> include =
input.getFragmentClasses().stream()
.filter((x) -> !exclude.contains(x))
.collect(toImmutableSet());
return input.trim(include);
}

/**
* Asserts that two configurations are the same, with exclusions.
*
* <p>Any fragments options of type specified in excludeFragmentOptions are excluded from the
* comparison.
*
* <p>Generally, this means they share the same checksum, which is computed by iterating over all
* the individual @Option annotated values contained within the {@link FragmentOption} classes
* contained within the {@link BuildOptions} inside the given configurations.
*/
protected void assertConfigurationsEqual(
BuildConfiguration config1,
BuildConfiguration config2,
Set<Class<? extends FragmentOptions>> excludeFragmentOptions) {
// BuildOptions and crosstool files determine a configuration's content. Within the context
// of these tests only the former actually change.
assertThat(config2.cloneOptions()).isEqualTo(config1.cloneOptions());

assertThat(trimConfiguration(config2.cloneOptions(), excludeFragmentOptions))
.isEqualTo(trimConfiguration(config1.cloneOptions(), excludeFragmentOptions));
}

protected void assertConfigurationsEqual(BuildConfiguration config1, BuildConfiguration config2) {
assertConfigurationsEqual(config1, config2, ImmutableSet.of());
}

/**
Expand Down Expand Up @@ -2210,7 +2237,7 @@ protected Iterable<String> baselineCoverageArtifactBasenames(ConfiguredTarget ta
baselineAction.newDeterministicWriter(ActionsTestUtil.createContext(reporter))
.writeOutputFile(bytes);

for (String line : new String(bytes.toByteArray(), StandardCharsets.UTF_8).split("\n")) {
for (String line : Splitter.on('\n').split(new String(bytes.toByteArray(), UTF_8))) {
if (line.startsWith("SF:")) {
String basename = line.substring(line.lastIndexOf('/') + 1);
basenames.add(basename);
Expand Down

0 comments on commit b7308f8

Please sign in to comment.