Skip to content

Commit 848d79d

Browse files
OmniLab Teamcopybara-github
authored andcommitted
Internal change
PiperOrigin-RevId: 915814833
1 parent c70f293 commit 848d79d

10 files changed

Lines changed: 110 additions & 20 deletions

File tree

src/java/com/google/devtools/mobileharness/fe/v6/service/device/handlers/ConfigurationButtonBuilder.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.devtools.mobileharness.api.query.proto.LabQueryProto.DeviceInfo;
2020
import com.google.devtools.mobileharness.fe.v6.service.proto.common.ActionButtonState;
2121
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManagerFactory;
22+
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureReadiness;
2223
import com.google.devtools.mobileharness.fe.v6.service.util.UniverseScope;
2324
import javax.inject.Inject;
2425
import javax.inject.Singleton;
@@ -28,10 +29,13 @@
2829
class ConfigurationButtonBuilder {
2930

3031
private final FeatureManagerFactory featureManagerFactory;
32+
private final FeatureReadiness featureReadiness;
3133

3234
@Inject
33-
ConfigurationButtonBuilder(FeatureManagerFactory featureManagerFactory) {
35+
ConfigurationButtonBuilder(
36+
FeatureManagerFactory featureManagerFactory, FeatureReadiness featureReadiness) {
3437
this.featureManagerFactory = featureManagerFactory;
38+
this.featureReadiness = featureReadiness;
3539
}
3640

3741
public ActionButtonState build(DeviceInfo deviceInfo, UniverseScope universe) {
@@ -41,7 +45,7 @@ public ActionButtonState build(DeviceInfo deviceInfo, UniverseScope universe) {
4145

4246
return ActionButtonState.newBuilder()
4347
.setVisible(true)
44-
.setIsReady(true)
48+
.setIsReady(featureReadiness.isDeviceConfigurationReady())
4549
.setEnabled(true)
4650
.setTooltip("Configure the device")
4751
.build();

src/java/com/google/devtools/mobileharness/fe/v6/service/host/handlers/HostConfigButtonBuilder.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.devtools.mobileharness.api.query.proto.LabQueryProto.LabInfo;
2020
import com.google.devtools.mobileharness.fe.v6.service.proto.common.ActionButtonState;
2121
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManagerFactory;
22+
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureReadiness;
2223
import com.google.devtools.mobileharness.fe.v6.service.util.UniverseScope;
2324
import java.util.Optional;
2425
import javax.inject.Inject;
@@ -29,10 +30,13 @@
2930
public class HostConfigButtonBuilder {
3031

3132
private final FeatureManagerFactory featureManagerFactory;
33+
private final FeatureReadiness featureReadiness;
3234

3335
@Inject
34-
HostConfigButtonBuilder(FeatureManagerFactory featureManagerFactory) {
36+
HostConfigButtonBuilder(
37+
FeatureManagerFactory featureManagerFactory, FeatureReadiness featureReadiness) {
3538
this.featureManagerFactory = featureManagerFactory;
39+
this.featureReadiness = featureReadiness;
3640
}
3741

3842
public ActionButtonState build(
@@ -44,6 +48,7 @@ public ActionButtonState build(
4448

4549
return ActionButtonState.newBuilder()
4650
.setVisible(true)
51+
.setIsReady(featureReadiness.isHostConfigurationReady())
4752
.setEnabled(true)
4853
.setTooltip("Configure the host configuration")
4954
.build();

src/java/com/google/devtools/mobileharness/fe/v6/service/util/FeatureManager.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public boolean isLabServerUpdatePassThroughFlagsFeatureEnabled() {
138138
}
139139

140140
/**
141-
* Checks if the configuration feature is enabled.
141+
* Checks if the configuration feature(for both devices and hosts) is enabled.
142142
*
143143
* <p>In Google internal builds, configuration is always enabled for {@link
144144
* UniverseScope.SelfUniverse} and disabled for routed universes. In OSS builds, availability is
@@ -150,6 +150,6 @@ public boolean isConfigurationFeatureEnabled() {
150150
return universe instanceof UniverseScope.SelfUniverse;
151151
}
152152
// OSS/ATS: availability depends on the flag.
153-
return Flags.feEnableConfiguration.getNonNull();
153+
return Flags.feConnectToConfigServer.getNonNull();
154154
}
155155
}

src/java/com/google/devtools/mobileharness/fe/v6/service/util/FeatureReadiness.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,21 @@
1616

1717
package com.google.devtools.mobileharness.fe.v6.service.util;
1818

19+
import javax.inject.Inject;
1920
import javax.inject.Singleton;
2021

2122
// TODO: Add test coverage for this file.
2223
/** Concrete class for checking if a feature is ready for use. Unready features return false. */
2324
@Singleton
2425
public class FeatureReadiness {
26+
27+
private final Environment environment;
28+
29+
@Inject
30+
FeatureReadiness(Environment environment) {
31+
this.environment = environment;
32+
}
33+
2534
public boolean isDeviceFlashingReady() {
2635
return false;
2736
}
@@ -73,4 +82,12 @@ public boolean isLabServerDeployReady() {
7382
public boolean isLabServerUpdatePassThroughFlagsReady() {
7483
return true;
7584
}
85+
86+
public boolean isHostConfigurationReady() {
87+
return !environment.isGoogleInternal();
88+
}
89+
90+
public boolean isDeviceConfigurationReady() {
91+
return !environment.isGoogleInternal();
92+
}
7693
}

src/java/com/google/devtools/mobileharness/shared/util/flags/Flags.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -906,14 +906,11 @@ public enum ConfigServiceStorageType {
906906

907907
@FlagSpec(
908908
name = "fe_connect_to_config_server",
909-
help = "Whether to connect to the config server in the FE server.")
909+
help =
910+
"Whether to connect to the config server in the FE server. When true, both host and"
911+
+ " device config will be enabled.")
910912
public static final Flag<Boolean> feConnectToConfigServer = Flag.value(false);
911913

912-
@FlagSpec(
913-
name = "fe_enable_configuration",
914-
help = "Enables the device configuration feature in the Mobile Harness FE UI.")
915-
public static final Flag<Boolean> feEnableConfiguration = Flag.value(false);
916-
917914
@FlagSpec(name = "fe_grpc_port", help = "gRPC port to listen on for FE servers.")
918915
public static final Flag<Integer> feGrpcPort = Flag.value(8080);
919916

src/javatests/com/google/devtools/mobileharness/fe/v6/service/device/handlers/ConfigurationButtonBuilderTest.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.devtools.mobileharness.fe.v6.service.proto.common.ActionButtonState;
2424
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManager;
2525
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManagerFactory;
26+
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureReadiness;
2627
import com.google.devtools.mobileharness.fe.v6.service.util.UniverseScope;
2728
import org.junit.Before;
2829
import org.junit.Rule;
@@ -40,12 +41,14 @@ public final class ConfigurationButtonBuilderTest {
4041
@Rule public final MockitoRule mocks = MockitoJUnit.rule();
4142
@Mock private FeatureManagerFactory featureManagerFactory;
4243
@Mock private FeatureManager featureManager;
44+
@Mock private FeatureReadiness featureReadiness;
4345

4446
private ConfigurationButtonBuilder configurationButtonBuilder;
4547

4648
@Before
4749
public void setUp() {
48-
configurationButtonBuilder = new ConfigurationButtonBuilder(featureManagerFactory);
50+
configurationButtonBuilder =
51+
new ConfigurationButtonBuilder(featureManagerFactory, featureReadiness);
4952
when(featureManagerFactory.create(SELF_UNIVERSE)).thenReturn(featureManager);
5053
}
5154

@@ -63,12 +66,27 @@ public void build_configurationDisabled_invisible() {
6366
@Test
6467
public void build_configurationEnabled_visibleEnabledWithTooltip() {
6568
when(featureManager.isConfigurationFeatureEnabled()).thenReturn(true);
69+
when(featureReadiness.isDeviceConfigurationReady()).thenReturn(true);
6670

6771
ActionButtonState state =
6872
configurationButtonBuilder.build(DeviceInfo.getDefaultInstance(), SELF_UNIVERSE);
6973

7074
assertThat(state.getVisible()).isTrue();
7175
assertThat(state.getEnabled()).isTrue();
76+
assertThat(state.getIsReady()).isTrue();
7277
assertThat(state.getTooltip()).isEqualTo("Configure the device");
7378
}
79+
80+
@Test
81+
public void build_configurationEnabledButNotReady_visibleEnabledNotReady() {
82+
when(featureManager.isConfigurationFeatureEnabled()).thenReturn(true);
83+
when(featureReadiness.isDeviceConfigurationReady()).thenReturn(false);
84+
85+
ActionButtonState state =
86+
configurationButtonBuilder.build(DeviceInfo.getDefaultInstance(), SELF_UNIVERSE);
87+
88+
assertThat(state.getVisible()).isTrue();
89+
assertThat(state.getEnabled()).isTrue();
90+
assertThat(state.getIsReady()).isFalse();
91+
}
7492
}

src/javatests/com/google/devtools/mobileharness/fe/v6/service/host/handlers/HostConfigButtonBuilderTest.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManager;
2323
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureManagerFactory;
24+
import com.google.devtools.mobileharness.fe.v6.service.util.FeatureReadiness;
2425
import com.google.devtools.mobileharness.fe.v6.service.util.UniverseScope;
2526
import java.util.Optional;
2627
import org.junit.Before;
@@ -39,27 +40,43 @@ public final class HostConfigButtonBuilderTest {
3940

4041
@Mock private FeatureManagerFactory mockFeatureManagerFactory;
4142
@Mock private FeatureManager mockFeatureManager;
43+
@Mock private FeatureReadiness mockFeatureReadiness;
4244

4345
private HostConfigButtonBuilder hostConfigButtonBuilder;
4446
private static final UniverseScope UNIVERSE = new UniverseScope.SelfUniverse();
4547

4648
@Before
4749
public void setUp() {
48-
hostConfigButtonBuilder = new HostConfigButtonBuilder(mockFeatureManagerFactory);
50+
hostConfigButtonBuilder =
51+
new HostConfigButtonBuilder(mockFeatureManagerFactory, mockFeatureReadiness);
4952
when(mockFeatureManagerFactory.create(UNIVERSE)).thenReturn(mockFeatureManager);
5053
}
5154

5255
@Test
5356
public void build_returnsExpectedState() {
5457
when(mockFeatureManager.isConfigurationFeatureEnabled()).thenReturn(true);
58+
when(mockFeatureReadiness.isHostConfigurationReady()).thenReturn(true);
5559

5660
var result = hostConfigButtonBuilder.build(UNIVERSE, Optional.empty(), Optional.empty());
5761

5862
assertThat(result.getVisible()).isTrue();
5963
assertThat(result.getEnabled()).isTrue();
64+
assertThat(result.getIsReady()).isTrue();
6065
assertThat(result.getTooltip()).isEqualTo("Configure the host configuration");
6166
}
6267

68+
@Test
69+
public void build_notReady_returnsNotReady() {
70+
when(mockFeatureManager.isConfigurationFeatureEnabled()).thenReturn(true);
71+
when(mockFeatureReadiness.isHostConfigurationReady()).thenReturn(false);
72+
73+
var result = hostConfigButtonBuilder.build(UNIVERSE, Optional.empty(), Optional.empty());
74+
75+
assertThat(result.getVisible()).isTrue();
76+
assertThat(result.getEnabled()).isTrue();
77+
assertThat(result.getIsReady()).isFalse();
78+
}
79+
6380
@Test
6481
public void build_configFeatureDisabled_returnsInvisible() {
6582
when(mockFeatureManager.isConfigurationFeatureEnabled()).thenReturn(false);

src/javatests/com/google/devtools/mobileharness/fe/v6/service/util/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ java_library(
2828
"//src/java/com/google/devtools/mobileharness/fe/v6/service/util",
2929
"//src/java/com/google/devtools/mobileharness/shared/util/flags/core:testing",
3030
"//src/javatests/com/google/devtools/mobileharness/builddefs:truth",
31-
"@maven//:com_google_guava_guava",
3231
"@maven//:junit_junit",
3332
"@maven//:org_mockito_mockito_core",
3433
],

src/javatests/com/google/devtools/mobileharness/fe/v6/service/util/FeatureManagerTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,15 @@ public void isDeviceFlashingFeatureEnabled_internal_google1p_returnsTrue() {
5151
@Test
5252
public void isConfigurationFeatureEnabled_scenario1_flagTrue_returnsTrue() {
5353
when(mockEnvironment.isGoogleInternal()).thenReturn(true);
54-
flags.set("fe_enable_configuration", "true");
54+
flags.set("fe_connect_to_config_server", "true");
5555
FeatureManager featureManager = new FeatureManager(mockEnvironment, SELF_UNIVERSE);
5656
assertThat(featureManager.isConfigurationFeatureEnabled()).isTrue();
5757
}
5858

5959
@Test
6060
public void isConfigurationFeatureEnabled_scenario1_flagFalse_returnsTrue() {
6161
when(mockEnvironment.isGoogleInternal()).thenReturn(true);
62-
flags.set("fe_enable_configuration", "false");
62+
flags.set("fe_connect_to_config_server", "false");
6363
FeatureManager featureManager = new FeatureManager(mockEnvironment, SELF_UNIVERSE);
6464
assertThat(featureManager.isConfigurationFeatureEnabled()).isTrue();
6565
}
@@ -76,7 +76,7 @@ public void isDeviceFlashingFeatureEnabled_internal_nonGoogle1p_returnsFalse() {
7676
@Test
7777
public void isConfigurationFeatureEnabled_scenario2_flagTrue_returnsFalse() {
7878
when(mockEnvironment.isGoogleInternal()).thenReturn(true);
79-
flags.set("fe_enable_configuration", "true");
79+
flags.set("fe_connect_to_config_server", "true");
8080
FeatureManager featureManager = new FeatureManager(mockEnvironment, ROUTED_UNIVERSE);
8181
assertThat(featureManager.isConfigurationFeatureEnabled()).isFalse();
8282
}
@@ -93,15 +93,15 @@ public void isDeviceFlashingFeatureEnabled_oss_returnsFalse() {
9393
@Test
9494
public void isConfigurationFeatureEnabled_scenario3_flagTrue_returnsTrue() {
9595
when(mockEnvironment.isGoogleInternal()).thenReturn(false);
96-
flags.set("fe_enable_configuration", "true");
96+
flags.set("fe_connect_to_config_server", "true");
9797
FeatureManager featureManager = new FeatureManager(mockEnvironment, ROUTED_UNIVERSE);
9898
assertThat(featureManager.isConfigurationFeatureEnabled()).isTrue();
9999
}
100100

101101
@Test
102102
public void isConfigurationFeatureEnabled_scenario3_flagFalse_returnsFalse() {
103103
when(mockEnvironment.isGoogleInternal()).thenReturn(false);
104-
flags.set("fe_enable_configuration", "false");
104+
flags.set("fe_connect_to_config_server", "false");
105105
FeatureManager featureManager = new FeatureManager(mockEnvironment, ROUTED_UNIVERSE);
106106
assertThat(featureManager.isConfigurationFeatureEnabled()).isFalse();
107107
}

src/javatests/com/google/devtools/mobileharness/fe/v6/service/util/FeatureReadinessTest.java

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,29 @@
1717
package com.google.devtools.mobileharness.fe.v6.service.util;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.mockito.Mockito.when;
2021

2122
import org.junit.Before;
23+
import org.junit.Rule;
2224
import org.junit.Test;
2325
import org.junit.runner.RunWith;
2426
import org.junit.runners.JUnit4;
27+
import org.mockito.Mock;
28+
import org.mockito.junit.MockitoJUnit;
29+
import org.mockito.junit.MockitoRule;
2530

2631
@RunWith(JUnit4.class)
2732
public final class FeatureReadinessTest {
2833

2934
private FeatureReadiness featureReadiness;
3035

36+
@Rule public final MockitoRule mocks = MockitoJUnit.rule();
37+
38+
@Mock private Environment environment;
39+
3140
@Before
3241
public void setUp() {
33-
featureReadiness = new FeatureReadiness();
42+
featureReadiness = new FeatureReadiness(environment);
3443
}
3544

3645
@Test
@@ -72,4 +81,28 @@ public void isHostDecommissionReady_returnsFalse() {
7281
public void isLabServerStartReady_returnsTrue() {
7382
assertThat(featureReadiness.isLabServerStartReady()).isTrue();
7483
}
84+
85+
@Test
86+
public void isHostConfigurationReady_internal_returnsFalse() {
87+
when(environment.isGoogleInternal()).thenReturn(true);
88+
assertThat(featureReadiness.isHostConfigurationReady()).isFalse();
89+
}
90+
91+
@Test
92+
public void isHostConfigurationReady_oss_returnsTrue() {
93+
when(environment.isGoogleInternal()).thenReturn(false);
94+
assertThat(featureReadiness.isHostConfigurationReady()).isTrue();
95+
}
96+
97+
@Test
98+
public void isDeviceConfigurationReady_internal_returnsFalse() {
99+
when(environment.isGoogleInternal()).thenReturn(true);
100+
assertThat(featureReadiness.isDeviceConfigurationReady()).isFalse();
101+
}
102+
103+
@Test
104+
public void isDeviceConfigurationReady_oss_returnsTrue() {
105+
when(environment.isGoogleInternal()).thenReturn(false);
106+
assertThat(featureReadiness.isDeviceConfigurationReady()).isTrue();
107+
}
75108
}

0 commit comments

Comments
 (0)