Skip to content

Commit 95654ec

Browse files
xaionaro@dx.centerxaionaro@dx.center
authored andcommitted
fix: run bindercli E2E tests on-device, exclude sub-interfaces from CLI
Two fixes: 1. E2E bindercli tests now exec the binary directly when running on-device instead of unconditionally skipping. Previously all 40 bindercli tests were skipped on-device; now 23 pass, 17 skip (services unavailable on the specific device). 2. spec2cli detects sub-interfaces (returned by methods on other interfaces but not registered with ServiceManager) and excludes them from CLI command generation. These interfaces cannot be discovered by findServiceByDescriptor, so the generated commands always failed with "no service with descriptor ... found".
1 parent 1817c72 commit 95654ec

2 files changed

Lines changed: 83 additions & 13 deletions

File tree

tests/e2e/aidlcli_test.go

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,22 @@ var emulatorStartedByTest bool
3737
// onDevice reports whether the test binary is running directly on an
3838
// Android device (i.e. /dev/binder is accessible). When true, the
3939
// emulator/adb setup is skipped — on-device tests open /dev/binder
40-
// directly, and aidlcli tests are skipped.
40+
// directly, and bindercli tests exec the binary directly.
4141
var onDevice bool
4242

43+
// bindercliAvailable is true when the bindercli binary exists at
44+
// deviceBinary. On-device, this requires the binary to have been
45+
// pushed separately; on-host it is built and pushed by TestMain.
46+
var bindercliAvailable bool
47+
4348
func TestMain(m *testing.M) {
4449
if _, err := os.Stat("/dev/binder"); err == nil {
4550
// Running on the device itself — skip emulator setup.
51+
// Bindercli tests will exec the binary directly if present.
4652
onDevice = true
53+
if _, err := os.Stat(deviceBinary); err == nil {
54+
bindercliAvailable = true
55+
}
4756
os.Exit(m.Run())
4857
}
4958

@@ -199,11 +208,23 @@ func logAndExit(msg string) {
199208
os.Exit(1)
200209
}
201210

202-
// runBindercli executes bindercli on the emulator via `adb -s <serial> shell`.
211+
// runBindercli executes bindercli and returns stdout, stderr, error.
212+
// When running on the host, it uses `adb -s <serial> shell` to execute
213+
// on the emulator. When running on-device, it execs the binary directly.
203214
// It always injects --format json for machine-parseable output.
204-
// Arguments are joined into a single shell command string to preserve
205-
// empty/quoted values across the adb shell boundary.
206215
func runBindercli(args ...string) (string, string, error) {
216+
if onDevice {
217+
fullArgs := append([]string{"--format", "json"}, args...)
218+
cmd := exec.Command(deviceBinary, fullArgs...)
219+
var stdout, stderr bytes.Buffer
220+
cmd.Stdout = &stdout
221+
cmd.Stderr = &stderr
222+
err := cmd.Run()
223+
return stdout.String(), stderr.String(), err
224+
}
225+
226+
// Host path: join into a single shell command string to preserve
227+
// empty/quoted values across the adb shell boundary.
207228
parts := make([]string, 0, len(args)+3)
208229
parts = append(parts, deviceBinary, "--format", "json")
209230
for _, a := range args {
@@ -230,12 +251,11 @@ func shellQuote(s string) string {
230251
return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
231252
}
232253

233-
// runBindercliOrSkip runs bindercli; skips the test when the service is unavailable
234-
// or when running directly on the device (aidlcli tests require adb from the host).
254+
// runBindercliOrSkip runs bindercli; skips the test when the service is unavailable.
235255
func runBindercliOrSkip(t *testing.T, args ...string) string {
236256
t.Helper()
237-
if onDevice {
238-
t.Skip("bindercli tests require adb from host; skipping on-device")
257+
if onDevice && !bindercliAvailable {
258+
t.Skip("bindercli binary not found on device at " + deviceBinary)
239259
}
240260
stdout, stderr, err := runBindercli(args...)
241261
if err != nil {
@@ -701,8 +721,8 @@ func runBindercliHALOrSkip(
701721
args ...string,
702722
) string {
703723
t.Helper()
704-
if onDevice {
705-
t.Skip("bindercli tests require adb from host; skipping on-device")
724+
if onDevice && !bindercliAvailable {
725+
t.Skip("bindercli binary not found on device at " + deviceBinary)
706726
}
707727
fullArgs := append([]string{serviceName}, args...)
708728
stdout, stderr, err := runBindercli(fullArgs...)
@@ -745,8 +765,8 @@ func TestBindercli_ServiceMethods(t *testing.T) {
745765
}
746766

747767
func TestBindercli_ServiceTransact(t *testing.T) {
748-
if onDevice {
749-
t.Skip("bindercli tests require adb from host; skipping on-device")
768+
if onDevice && !bindercliAvailable {
769+
t.Skip("bindercli binary not found on device at " + deviceBinary)
750770
}
751771

752772
// SurfaceFlinger code 1022 (getSchedulingPolicy) is a simple read-only

tools/cmd/spec2cli/main.go

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,12 @@ var knownServiceNames map[string]string
122122
// resolving interface types (kindInterfaceType) in other packages.
123123
var knownInterfaces map[string]*interfaceInfo
124124

125+
// subInterfaceDescriptors is the set of AIDL descriptors that are
126+
// returned by methods on other interfaces but are NOT registered with
127+
// ServiceManager. These cannot be discovered by findServiceByDescriptor
128+
// and are excluded from top-level CLI command generation.
129+
var subInterfaceDescriptors map[string]bool
130+
125131
func main() {
126132
specsDir := flag.String("specs", "specs/", "Directory containing spec YAML files")
127133
outputDir := flag.String("output", "cmd/bindercli/", "Output directory for generated files")
@@ -162,8 +168,34 @@ func run(
162168
fmt.Fprintf(os.Stderr, "Scanned enum types: %d\n", len(knownEnums))
163169
fmt.Fprintf(os.Stderr, "Known service mappings: %d\n", len(knownServiceNames))
164170

165-
// Phase 2: build interface list.
171+
// Phase 2: build interface list and detect sub-interfaces.
172+
//
173+
// A sub-interface is one that appears as a return type of a method on
174+
// another interface but is not registered with ServiceManager. These
175+
// cannot be discovered via findServiceByDescriptor and should not get
176+
// top-level CLI commands.
166177
var interfaces []*interfaceInfo
178+
179+
// Collect all interface descriptors that appear as method return types.
180+
// These are potential sub-interfaces (obtained via a parent, not via
181+
// ServiceManager).
182+
returnedDescriptors := map[string]bool{}
183+
for _, ps := range specs {
184+
for _, iface := range ps.Interfaces {
185+
for _, m := range iface.Methods {
186+
rt := m.ReturnType.Name
187+
if rt == "" {
188+
continue
189+
}
190+
// Qualify unqualified names with the current package.
191+
if !strings.Contains(rt, ".") && ps.AIDLPackage != "" {
192+
rt = ps.AIDLPackage + "." + rt
193+
}
194+
returnedDescriptors[rt] = true
195+
}
196+
}
197+
}
198+
167199
for _, ps := range specs {
168200
importPath := modulePath + "/" + ps.GoPackage
169201
pkgName := filepath.Base(ps.GoPackage)
@@ -179,6 +211,19 @@ func run(
179211
return interfaces[i].Descriptor < interfaces[j].Descriptor
180212
})
181213

214+
// Identify sub-interfaces: descriptors that appear as method return
215+
// types but have no ServiceManager registration.
216+
subInterfaceDescriptors = map[string]bool{}
217+
for _, ii := range interfaces {
218+
if _, hasMapping := knownServiceNames[ii.Descriptor]; hasMapping {
219+
continue
220+
}
221+
if returnedDescriptors[ii.Descriptor] {
222+
subInterfaceDescriptors[ii.Descriptor] = true
223+
}
224+
}
225+
fmt.Fprintf(os.Stderr, "Sub-interfaces (excluded from CLI): %d\n", len(subInterfaceDescriptors))
226+
182227
totalMethods := 0
183228
commandableMethods := 0
184229
for _, iface := range interfaces {
@@ -1009,6 +1054,11 @@ func writeCommandsGen(
10091054
if iface.ImportPath == modulePath {
10101055
continue
10111056
}
1057+
// Skip sub-interfaces: they are obtained via methods on other
1058+
// interfaces and cannot be discovered by findServiceByDescriptor.
1059+
if subInterfaceDescriptors[iface.Descriptor] {
1060+
continue
1061+
}
10121062

10131063
var cmdMethods []methodInfo
10141064
for _, m := range iface.Methods {

0 commit comments

Comments
 (0)