Download even if an object is not part of the global scope
We should resolve gs:// path in @Option even if the object
is not part of the global scope.
This ensure that alias and global_namespace object are
properly downloaded and resolved.
Test: unit tests
Bug: 119414945
Change-Id: Ie2670b6886cba8c17ef2db9a3ac20c31bbf6f177
diff --git a/src/com/android/tradefed/config/DynamicRemoteFileResolver.java b/src/com/android/tradefed/config/DynamicRemoteFileResolver.java
index 8667f6d..d051576 100644
--- a/src/com/android/tradefed/config/DynamicRemoteFileResolver.java
+++ b/src/com/android/tradefed/config/DynamicRemoteFileResolver.java
@@ -63,12 +63,7 @@
Set<File> downloadedFiles = new HashSet<>();
try {
for (Map.Entry<String, OptionFieldsForName> optionPair : mOptionMap.entrySet()) {
- final String optName = optionPair.getKey();
final OptionFieldsForName optionFields = optionPair.getValue();
- if (optName.indexOf(OptionSetter.NAMESPACE_SEPARATOR) >= 0) {
- // Only return unqualified option names
- continue;
- }
for (Map.Entry<Object, Field> fieldEntry : optionFields) {
final Object obj = fieldEntry.getKey();
final Field field = fieldEntry.getValue();
diff --git a/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java b/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
index 920f454..7f0cc62 100644
--- a/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
+++ b/tests/src/com/android/tradefed/config/DynamicRemoteFileResolverTest.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import com.android.tradefed.config.remote.GcsRemoteFileResolver;
import com.android.tradefed.config.remote.IRemoteFileResolver;
import com.android.tradefed.util.FileUtil;
@@ -46,6 +47,12 @@
public Collection<File> remoteFileList = new ArrayList<>();
}
+ @OptionClass(alias = "option-class-alias", global_namespace = false)
+ private static class RemoteFileOptionWithOptionClass {
+ @Option(name = "remote-file")
+ public File remoteFile = null;
+ }
+
private DynamicRemoteFileResolver mResolver;
private IRemoteFileResolver mMockResolver;
@@ -56,7 +63,10 @@
new DynamicRemoteFileResolver() {
@Override
IRemoteFileResolver getResolver(String protocol) {
- return mMockResolver;
+ if (protocol.equals(GcsRemoteFileResolver.PROTOCOL)) {
+ return mMockResolver;
+ }
+ return null;
}
};
}
@@ -116,10 +126,6 @@
EasyMock.expect(
mMockResolver.resolveRemoteFiles(
- EasyMock.eq(new File("fake/file")), EasyMock.anyObject()))
- .andReturn(null);
- EasyMock.expect(
- mMockResolver.resolveRemoteFiles(
EasyMock.eq(new File("gs:/fake/path")), EasyMock.anyObject()))
.andReturn(fake);
EasyMock.replay(mMockResolver);
@@ -162,10 +168,6 @@
File fake = FileUtil.createTempFile("gs-option-setter-test", "txt");
EasyMock.expect(
mMockResolver.resolveRemoteFiles(
- EasyMock.eq(new File("fake/file")), EasyMock.anyObject()))
- .andReturn(fake);
- EasyMock.expect(
- mMockResolver.resolveRemoteFiles(
EasyMock.eq(new File("gs://success/fake/path")),
EasyMock.anyObject()))
.andReturn(fake);
@@ -188,4 +190,42 @@
}
EasyMock.verify(mMockResolver);
}
+
+ @Test
+ public void testResolve_withNoGlobalNameSpace() throws Exception {
+ RemoteFileOptionWithOptionClass object = new RemoteFileOptionWithOptionClass();
+ OptionSetter setter =
+ new OptionSetter(object) {
+ @Override
+ DynamicRemoteFileResolver createResolver() {
+ return mResolver;
+ }
+ };
+
+ File fake = FileUtil.createTempFile("gs-option-setter-test", "txt");
+
+ setter.setOptionValue("option-class-alias:remote-file", "gs://fake/path");
+ assertEquals("gs:/fake/path", object.remoteFile.getPath());
+
+ // File is downloaded the first time, then is ignored since it doesn't have the protocol
+ // anymore
+ EasyMock.expect(
+ mMockResolver.resolveRemoteFiles(
+ EasyMock.eq(new File("gs:/fake/path")), EasyMock.anyObject()))
+ .andReturn(fake);
+ EasyMock.replay(mMockResolver);
+
+ Set<File> downloadedFile = setter.validateRemoteFilePath();
+ try {
+ assertEquals(1, downloadedFile.size());
+ File downloaded = downloadedFile.iterator().next();
+ // The file has been replaced by the downloaded one.
+ assertEquals(downloaded.getAbsolutePath(), object.remoteFile.getAbsolutePath());
+ } finally {
+ for (File f : downloadedFile) {
+ FileUtil.recursiveDelete(f);
+ }
+ }
+ EasyMock.verify(mMockResolver);
+ }
}