Ensure replicate do not attempt to resolve build_provider again
We don't expect any download from build_provider replicat since
we link the build info from the original one.
Test: unit tests
Bug: 152226657
Change-Id: I3a38491fa44577e3a93f3adacbe870934d8ed7ba
diff --git a/src/com/android/tradefed/build/BootstrapBuildProvider.java b/src/com/android/tradefed/build/BootstrapBuildProvider.java
index 919a26a..5cd19e8 100644
--- a/src/com/android/tradefed/build/BootstrapBuildProvider.java
+++ b/src/com/android/tradefed/build/BootstrapBuildProvider.java
@@ -143,4 +143,8 @@
ExecutionFiles getInvocationFiles() {
return CurrentInvocation.getInvocationFiles();
}
+
+ public final File getTestsDir() {
+ return mTestsDir;
+ }
}
diff --git a/src/com/android/tradefed/config/OptionSetter.java b/src/com/android/tradefed/config/OptionSetter.java
index bd10951..ec17767 100644
--- a/src/com/android/tradefed/config/OptionSetter.java
+++ b/src/com/android/tradefed/config/OptionSetter.java
@@ -37,6 +37,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -253,7 +254,7 @@
*/
protected class OptionFieldsForName implements Iterable<Map.Entry<Object, Field>> {
- private Map<Object, Field> mSourceFieldMap = new HashMap<Object, Field>();
+ private Map<Object, Field> mSourceFieldMap = new LinkedHashMap<Object, Field>();
void addField(String name, Object source, Field field) throws ConfigurationException {
if (size() > 0) {
@@ -561,7 +562,7 @@
private Map<String, OptionFieldsForName> makeOptionMap() throws ConfigurationException {
final Map<String, Integer> freqMap = new HashMap<String, Integer>(mOptionSources.size());
final Map<String, OptionFieldsForName> optionMap =
- new HashMap<String, OptionFieldsForName>();
+ new LinkedHashMap<String, OptionFieldsForName>();
for (Object objectSource : mOptionSources) {
final String className = objectSource.getClass().getName();
diff --git a/src/com/android/tradefed/invoker/TestInvocation.java b/src/com/android/tradefed/invoker/TestInvocation.java
index 7cb0570..46edad7 100644
--- a/src/com/android/tradefed/invoker/TestInvocation.java
+++ b/src/com/android/tradefed/invoker/TestInvocation.java
@@ -628,12 +628,13 @@
RunMode mode) {
try {
// Don't resolve for remote invocation, wait until we are inside the remote.
- if (!RunMode.REMOTE_INVOCATION.equals(mode)) {
- DynamicRemoteFileResolver resolver = new DynamicRemoteFileResolver();
- resolver.setDevice(context.getDevices().get(0));
- resolver.addExtraArgs(config.getCommandOptions().getDynamicDownloadArgs());
- config.resolveDynamicOptions(resolver);
+ if (RunMode.REMOTE_INVOCATION.equals(mode)) {
+ return true;
}
+ DynamicRemoteFileResolver resolver = new DynamicRemoteFileResolver();
+ resolver.setDevice(context.getDevices().get(0));
+ resolver.addExtraArgs(config.getCommandOptions().getDynamicDownloadArgs());
+ config.resolveDynamicOptions(resolver);
return true;
} catch (RuntimeException | BuildRetrievalError | ConfigurationException e) {
// In case of build not found issues.
diff --git a/src/com/android/tradefed/invoker/shard/ParentShardReplicate.java b/src/com/android/tradefed/invoker/shard/ParentShardReplicate.java
index 3d78483..1188a17 100644
--- a/src/com/android/tradefed/invoker/shard/ParentShardReplicate.java
+++ b/src/com/android/tradefed/invoker/shard/ParentShardReplicate.java
@@ -16,6 +16,7 @@
package com.android.tradefed.invoker.shard;
import com.android.ddmlib.Log.LogLevel;
+import com.android.tradefed.build.StubBuildProvider;
import com.android.tradefed.config.Configuration;
import com.android.tradefed.config.ConfigurationException;
import com.android.tradefed.config.IConfiguration;
@@ -56,6 +57,8 @@
String newName = String.format("expanded-%s", i);
IDeviceConfiguration newDeviceConfig =
deepCopy.getDeviceConfig().get(0).clone(newName);
+ // Stub the build provider since it should never be called
+ newDeviceConfig.addSpecificConfig(new StubBuildProvider());
currentConfigs.add(newDeviceConfig);
}
config.setDeviceConfigList(currentConfigs);
diff --git a/tests/res/config/tf/bootstrap.xml b/tests/res/config/tf/bootstrap.xml
new file mode 100644
index 0000000..f868565
--- /dev/null
+++ b/tests/res/config/tf/bootstrap.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2020 The Android Open Source Project
+
+ 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.
+-->
+<configuration description="test config for unit tests">
+ <build_provider class="com.android.tradefed.build.BootstrapBuildProvider" />
+</configuration>
diff --git a/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java b/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
index c4e54e7..a432d86 100644
--- a/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
+++ b/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
@@ -23,9 +23,12 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import com.android.tradefed.build.BootstrapBuildProvider;
import com.android.tradefed.build.BuildRetrievalError;
+import com.android.tradefed.build.StubBuildProvider;
import com.android.tradefed.config.remote.GcsRemoteFileResolver;
import com.android.tradefed.config.remote.IRemoteFileResolver;
+import com.android.tradefed.invoker.shard.ParentShardReplicate;
import com.android.tradefed.util.FileUtil;
import com.android.tradefed.util.MultiMap;
@@ -563,4 +566,96 @@
// since we should always have a few.
assertThat(listResolver).isNotEmpty();
}
+
+ @Test
+ public void testMultiDevices() throws Exception {
+ IConfiguration configuration = new Configuration("test", "test");
+
+ List<IDeviceConfiguration> listConfigs = new ArrayList<>();
+ IDeviceConfiguration holder1 = new DeviceConfigurationHolder("device1");
+ BootstrapBuildProvider provider1 = new BootstrapBuildProvider();
+ OptionSetter setter = new OptionSetter(provider1);
+ setter.setOptionValue("tests-dir", "gs://fake/path");
+ holder1.addSpecificConfig(provider1);
+ listConfigs.add(holder1);
+
+ IDeviceConfiguration holder2 = new DeviceConfigurationHolder("device2");
+ BootstrapBuildProvider provider2 = new BootstrapBuildProvider();
+ OptionSetter setter2 = new OptionSetter(provider2);
+ setter2.setOptionValue("tests-dir", "gs://fake/path");
+ holder2.addSpecificConfig(provider2);
+ listConfigs.add(holder2);
+
+ configuration.setDeviceConfigList(listConfigs);
+
+ File fake = FileUtil.createTempFile("gs-option-setter-test", "txt");
+ File fake2 = FileUtil.createTempFile("gs-option-setter-test", "txt");
+
+ EasyMock.expect(
+ mMockResolver.resolveRemoteFiles(
+ EasyMock.eq(new File("gs:/fake/path")), EasyMock.anyObject()))
+ .andReturn(fake);
+ EasyMock.expect(
+ mMockResolver.resolveRemoteFiles(
+ EasyMock.eq(new File("gs:/fake/path")), EasyMock.anyObject()))
+ .andReturn(fake2);
+
+ EasyMock.replay(mMockResolver);
+ configuration.resolveDynamicOptions(mResolver);
+ try {
+ assertEquals(fake, provider1.getTestsDir());
+ assertEquals(fake2, provider2.getTestsDir());
+ } finally {
+ configuration.cleanConfigurationData();
+ }
+ EasyMock.verify(mMockResolver);
+ }
+
+ @Test
+ public void testMultiDevices_replicat() throws Exception {
+ IConfiguration configuration =
+ new Configuration("test", "test") {
+ @Override
+ protected boolean isRemoteEnvironment() {
+ return true;
+ }
+ };
+ configuration.getCommandOptions().setReplicateSetup(true);
+ configuration.getCommandOptions().setShardCount(2);
+ configuration.setCommandLine(
+ new String[] {"tf/bootstrap", "--tests-dir", "gs://fake/path"});
+
+ List<IDeviceConfiguration> listConfigs = new ArrayList<>();
+ IDeviceConfiguration holder1 =
+ new DeviceConfigurationHolder(ConfigurationDef.DEFAULT_DEVICE_NAME);
+ BootstrapBuildProvider provider1 = new BootstrapBuildProvider();
+ OptionSetter setter = new OptionSetter(provider1);
+ setter.setOptionValue("tests-dir", "gs://fake/path");
+ holder1.addSpecificConfig(provider1);
+ listConfigs.add(holder1);
+
+ configuration.setDeviceConfigList(listConfigs);
+
+ assertEquals(1, configuration.getDeviceConfig().size());
+ ParentShardReplicate.replicatedSetup(configuration, null);
+ assertEquals(2, configuration.getDeviceConfig().size());
+
+ assertTrue(
+ configuration.getDeviceConfig().get(1).getBuildProvider()
+ instanceof StubBuildProvider);
+
+ File fake = FileUtil.createTempFile("gs-option-setter-test", ".txt");
+ EasyMock.expect(
+ mMockResolver.resolveRemoteFiles(
+ EasyMock.eq(new File("gs:/fake/path")), EasyMock.anyObject()))
+ .andReturn(fake);
+ EasyMock.replay(mMockResolver);
+ configuration.resolveDynamicOptions(mResolver);
+ try {
+ assertEquals(fake, provider1.getTestsDir());
+ } finally {
+ configuration.cleanConfigurationData();
+ }
+ EasyMock.verify(mMockResolver);
+ }
}