diff --git a/clients/dotnet/Mobium/Device.cs b/clients/dotnet/Mobium/Device.cs index 169eba3..a1021b2 100644 --- a/clients/dotnet/Mobium/Device.cs +++ b/clients/dotnet/Mobium/Device.cs @@ -1462,9 +1462,10 @@ public void Dispose() /// /// Ends the session on the device and closes the connection. The teardown is the daemon's own: accessibility - /// settings put back, a recording or route stopped, WebViews detached, + /// settings put back, a route stopped, WebViews detached, /// the device-side server stopped, and the app Start launched, if any, - /// stopped too. Quitting a session that is not open succeeds, and a + /// stopped too. A running recording or audio capture is discarded, with + /// a line on standard error. Quitting a session that is not open succeeds, and a /// second quit — a using block ending after an explicit one — /// does nothing. A program that exits without quitting or disposing /// has the sessions it started ended for it: mobium sees the client go. @@ -1477,7 +1478,12 @@ public void Quit() if (Session != null) args["device"] = Session.Device; try { - _conn.Call("app_session", args); + var ended = Data("app_session", args); + if (ended.TryGetValue("discarded", out var discarded) && discarded is IList names) + { + foreach (var what in Strings(names)) + Console.Error.WriteLine($"mobium: {what} was discarded, unsaved: stop it before Quit() to keep it"); + } } finally { diff --git a/clients/dotnet/Tests/FakeMobium.cs b/clients/dotnet/Tests/FakeMobium.cs index 2edfa14..ee29b81 100644 --- a/clients/dotnet/Tests/FakeMobium.cs +++ b/clients/dotnet/Tests/FakeMobium.cs @@ -107,6 +107,7 @@ internal static int Run() { view["device"] = (string?)args?["device"] ?? ""; view["ended"] = true; + if (mode == "discards") view["discarded"] = new JsonArray("the screen recording"); } Reply(stdout, id, new JsonObject { diff --git a/clients/dotnet/Tests/Program.cs b/clients/dotnet/Tests/Program.cs index 69c860a..af172ec 100644 --- a/clients/dotnet/Tests/Program.cs +++ b/clients/dotnet/Tests/Program.cs @@ -66,6 +66,7 @@ private static int Main(string[] args) DisposeIsIdempotentAndEndsAWaitingCall(); AnAnswerWithNoIdFailsTheCallInFlight(); StartOpensTheSessionAndQuitEndsIt(); + QuitWarnsAboutWhatItDiscarded(); UploadSendsOnlyWhatWasGiven(); DownloadSavesDecodesOrLists(); MapDiffSendsDiffAndReadsWhatChanged(); @@ -611,6 +612,27 @@ private static void AnAnswerWithNoIdFailsTheCallInFlight() Eq("and the connection is still in step", "app_current", Json.Str(d.Call("app_current", null), "tool")); } + private static void QuitWarnsAboutWhatItDiscarded() + { + var stderr = Console.Error; + var written = new StringWriter(); + Console.SetError(written); + try + { + Environment.SetEnvironmentVariable("MOBIUM_FAKE", "discards"); + Device.Builder().Binary(FakeBinary()).Platform("android").Start().Quit(); + Yes("quit warns about the recording", written.ToString().Contains("the screen recording was discarded")); + written.GetStringBuilder().Clear(); + Environment.SetEnvironmentVariable("MOBIUM_FAKE", "ok"); + Device.Builder().Binary(FakeBinary()).Platform("android").Start().Quit(); + Yes("quit warns about nothing when nothing was discarded", written.ToString().Length == 0); + } + finally + { + Console.SetError(stderr); + } + } + private static void StartOpensTheSessionAndQuitEndsIt() { Environment.SetEnvironmentVariable("MOBIUM_FAKE", "ok"); diff --git a/clients/go/client_test.go b/clients/go/client_test.go index 6fbdbd7..3aa60a7 100644 --- a/clients/go/client_test.go +++ b/clients/go/client_test.go @@ -2,10 +2,12 @@ package mobium import ( "bufio" + "bytes" "context" "encoding/json" "errors" "fmt" + "log" "os" "path/filepath" "runtime" @@ -146,7 +148,11 @@ func fakeTool(name string, args map[string]any, scenario string) map[string]any return text("session started", map[string]any{"action": "start", "device": "fake-device", "platform": platform, "driver": driver, "app": app, "sessions": []any{}}) case "end": - return text("session ended", map[string]any{"action": "end", "device": args["device"], "ended": true, "sessions": []any{}}) + view := map[string]any{"action": "end", "device": args["device"], "ended": true, "sessions": []any{}} + if os.Getenv(mobiumFakeEnv) == "discards" { + view["discarded"] = []any{"the screen recording"} + } + return text("session ended", view) default: return text("no session is open", map[string]any{"action": "status", "sessions": []any{}}) } @@ -644,6 +650,38 @@ func TestStartOpensTheSessionAndQuitEndsIt(t *testing.T) { } } +func TestQuitSaysWhatItDiscarded(t *testing.T) { + ctx := context.Background() + var logged bytes.Buffer + log.SetOutput(&logged) + defer log.SetOutput(os.Stderr) + + t.Setenv(mobiumFakeEnv, "discards") + dev, err := Start(ctx, WithBinary(os.Args[0])) + if err != nil { + t.Fatalf("start: %v", err) + } + if err := dev.Quit(ctx); err != nil { + t.Fatalf("quit: %v", err) + } + if !strings.Contains(logged.String(), "the screen recording was discarded") { + t.Errorf("logged %q", logged.String()) + } + + logged.Reset() + t.Setenv(mobiumFakeEnv, "1") + dev, err = Start(ctx, WithBinary(os.Args[0])) + if err != nil { + t.Fatalf("start: %v", err) + } + if err := dev.Quit(ctx); err != nil { + t.Fatalf("quit: %v", err) + } + if logged.Len() != 0 { + t.Errorf("logged %q with nothing discarded", logged.String()) + } +} + func TestADeviceFromConnectHasNoSession(t *testing.T) { dev := connectFake(t, "") if dev.Session() != nil { diff --git a/clients/go/mobium.go b/clients/go/mobium.go index 8d4341c..367f1f0 100644 --- a/clients/go/mobium.go +++ b/clients/go/mobium.go @@ -39,6 +39,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "os" "strings" "time" @@ -216,6 +217,9 @@ func (d *Device) Session() *Session { return d.started } // too. Quitting a session that is not open succeeds, and a second Quit — a // deferred one after an explicit one, say — does nothing. // +// A running recording or audio capture is discarded, and said so through the +// standard logger. +// // A program that exits without Quit or Close has the sessions it started // ended for it, the same way: mobium sees the client go. func (d *Device) Quit(ctx context.Context) error { @@ -227,7 +231,13 @@ func (d *Device) Quit(ctx context.Context) error { if d.started != nil { args["device"] = d.started.Device } - err := d.act(ctx, "app_session", args) + var ended struct { + Discarded []string `json:"discarded"` + } + err := d.data(ctx, "app_session", args, &ended) + for _, what := range ended.Discarded { + log.Printf("mobium: %s was discarded, unsaved: stop it before Quit to keep it", what) + } if cerr := d.Close(); err == nil { err = cerr } diff --git a/clients/java/src/main/java/dev/mobium/Mobium.java b/clients/java/src/main/java/dev/mobium/Mobium.java index 3f757a3..6235243 100644 --- a/clients/java/src/main/java/dev/mobium/Mobium.java +++ b/clients/java/src/main/java/dev/mobium/Mobium.java @@ -9,6 +9,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.logging.Logger; /** * Drives mobile apps on Android emulators, Android phones, iOS simulators @@ -39,6 +40,8 @@ */ public final class Mobium implements AutoCloseable { + private static final Logger LOG = Logger.getLogger("dev.mobium"); + private final Connection conn; // What start() opened, so quit() ends that device's session even when // several are running. Null after connect(). @@ -2258,8 +2261,10 @@ public Map call(String tool, Map arguments) { * server stopped, and the app {@code start()} launched, if any, stopped * too. Quitting a session that is not open succeeds, and a second quit -- * say, try-with-resources closing after an explicit one -- does nothing. - * A program that exits without quitting or closing has the sessions it - * started ended for it: mobium sees the client go. + * A running recording or audio capture is discarded, with a warning on + * the {@code dev.mobium} logger. A program that exits without quitting or + * closing has the sessions it started ended for it: mobium sees the client + * go. */ public void quit() { if (quit) return; @@ -2268,7 +2273,9 @@ public void quit() { args.put("action", "end"); if (session != null) args.put("device", session.device()); try { - conn.call("app_session", args); + for (Object what : Json.asArray(data("app_session", args).get("discarded"))) { + LOG.warning(what + " was discarded, unsaved: stop it before quit() to keep it"); + } } finally { conn.close(); } diff --git a/clients/java/src/test/java/dev/mobium/FakeMobium.java b/clients/java/src/test/java/dev/mobium/FakeMobium.java index b3107d8..8e6a1aa 100644 --- a/clients/java/src/test/java/dev/mobium/FakeMobium.java +++ b/clients/java/src/test/java/dev/mobium/FakeMobium.java @@ -110,6 +110,7 @@ public static void main(String[] args) throws Exception { } else if (action.equals("end")) { view.put("device", Json.str(a, "device")); view.put("ended", true); + if (mode.equals("discards")) view.put("discarded", List.of("the screen recording")); } view.put("sessions", List.of()); reply(out, id, Map.of("content", List.of(Map.of("type", "text", "text", "session")), "structuredContent", view)); diff --git a/clients/java/src/test/java/dev/mobium/Tests.java b/clients/java/src/test/java/dev/mobium/Tests.java index ae0742d..5dd408e 100644 --- a/clients/java/src/test/java/dev/mobium/Tests.java +++ b/clients/java/src/test/java/dev/mobium/Tests.java @@ -62,6 +62,7 @@ public static void main(String[] args) { closeIsIdempotentAndEndsAWaitingCall(); anAnswerWithNoIdFailsTheCallInFlight(); startOpensTheSessionAndQuitEndsIt(); + quitWarnsAboutWhatItDiscarded(); filesSendTheirArgumentsAndDecodeTheAnswer(); mapDiffSendsDiffAndReturnsTheDiff(); @@ -475,6 +476,29 @@ static void anAnswerWithNoIdFailsTheCallInFlight() { } } + static void quitWarnsAboutWhatItDiscarded() { + List warned = new ArrayList<>(); + java.util.logging.Logger log = java.util.logging.Logger.getLogger("dev.mobium"); + java.util.logging.Handler handler = new java.util.logging.Handler() { + @Override public void publish(java.util.logging.LogRecord r) { warned.add(r.getMessage()); } + @Override public void flush() { } + @Override public void close() { } + }; + log.addHandler(handler); + log.setUseParentHandlers(false); + try { + Mobium.builder().binary(FakeProcess.launcher("discards", null)).platform("android").start().quit(); + yes("quit warns about the recording", + warned.size() == 1 && warned.get(0).contains("the screen recording was discarded")); + warned.clear(); + Mobium.builder().binary(FakeProcess.launcher("ok", null)).platform("android").start().quit(); + yes("quit warns about nothing when nothing was discarded", warned.isEmpty()); + } finally { + log.removeHandler(handler); + log.setUseParentHandlers(true); + } + } + static void startOpensTheSessionAndQuitEndsIt() { String fake = FakeProcess.launcher("ok", null); Mobium d = Mobium.builder().binary(fake).platform("ios").app("com.apple.Preferences").start(); diff --git a/clients/javascript/index.js b/clients/javascript/index.js index 4a64be1..ec9fce8 100644 --- a/clients/javascript/index.js +++ b/clients/javascript/index.js @@ -1828,7 +1828,8 @@ export class Device { /** * Ends the session on the device and closes the connection. The teardown is the daemon's own: accessibility settings put * back, a recording or route stopped, WebViews detached, the device-side - * server stopped, and the app start() launched, if any, stopped too. + * server stopped, and the app start() launched, if any, stopped too. A + * running recording or audio capture is discarded, with a MobiumWarning. * Quitting a session that is not open succeeds, and a second quit does * nothing. A script that exits without quit() or close() has the sessions * it started ended for it: mobium sees the client go. @@ -1840,7 +1841,10 @@ export class Device { const args = { action: 'end' } if (this.session) args.device = this.session.device try { - await this.conn.callTool('app_session', args) + const ended = (await this.conn.callTool('app_session', args)).structuredContent || {} + for (const what of ended.discarded || []) { + process.emitWarning(`${what} was discarded, unsaved: stop it before quit() to keep it`, 'MobiumWarning') + } } finally { await this.close() } diff --git a/clients/javascript/test/connection.test.mjs b/clients/javascript/test/connection.test.mjs index 6513536..38dee2c 100644 --- a/clients/javascript/test/connection.test.mjs +++ b/clients/javascript/test/connection.test.mjs @@ -144,6 +144,24 @@ const within = (p, ms) => Promise.race([settle(p), new Promise((r) => setTimeout check(after.ok === false && /closed/.test(after.e.message), `a call after close does not say it was closed: ${JSON.stringify(after)}`) } +// -- quit warns about what it discarded ------------------------------------- +{ + const warned = [] + const listen = (w) => { if (w.name === 'MobiumWarning') warned.push(w.message) } + process.on('warning', listen) + process.env.MOBIUM_FAKE = 'discards' + await (await m.start({ platform: 'android', binary: FAKE })).quit() + await new Promise((resolve) => setImmediate(resolve)) + check(warned.length === 1 && warned[0].includes('the screen recording was discarded'), + `quit did not warn about the recording: ${JSON.stringify(warned)}`) + warned.length = 0 + process.env.MOBIUM_FAKE = 'ok' + await (await m.start({ platform: 'android', binary: FAKE })).quit() + await new Promise((resolve) => setImmediate(resolve)) + check(warned.length === 0, `quit warned with nothing discarded: ${JSON.stringify(warned)}`) + process.off('warning', listen) +} + // -- start opens the session, quit ends it -------------------------------- { process.env.MOBIUM_FAKE = 'ok' diff --git a/clients/javascript/test/fake-mobium.mjs b/clients/javascript/test/fake-mobium.mjs index b5cd4b0..e7c82db 100644 --- a/clients/javascript/test/fake-mobium.mjs +++ b/clients/javascript/test/fake-mobium.mjs @@ -53,6 +53,7 @@ async function handle(msg) { view = { action: 'start', device: 'fake-device', platform, driver: platform === 'ios' ? 'wda' : 'uiautomator2', app: args.app || '', sessions: [] } } else if (args.action === 'end') { view = { action: 'end', device: args.device || '', ended: true, sessions: [] } + if (mode === 'discards') view.discarded = ['the screen recording'] } else { view = { action: 'status', sessions: [] } } diff --git a/clients/python/mobium/__init__.py b/clients/python/mobium/__init__.py index 9bea036..d671260 100644 --- a/clients/python/mobium/__init__.py +++ b/clients/python/mobium/__init__.py @@ -8,7 +8,7 @@ device.tap("@e1") """ -from ._device import Bounds, Device, DeviceInfo, Element, Session, connect, start +from ._device import Bounds, Device, DeviceInfo, Element, MobiumWarning, Session, connect, start from ._errors import ( AmbiguousLocatorError, DeviceNotReadyError, diff --git a/clients/python/mobium/_device.py b/clients/python/mobium/_device.py index 3d30887..a65f099 100644 --- a/clients/python/mobium/_device.py +++ b/clients/python/mobium/_device.py @@ -3,12 +3,22 @@ from __future__ import annotations import base64 +import warnings from dataclasses import dataclass from typing import Any from ._rpc import Connection, MobiumError, find_binary, _data_of, _text_of +class MobiumWarning(UserWarning): + """Something was lost that nothing raised for: a recording quit() discarded.""" + + +# Python shows a warning once per place by default, and every recording a +# process lost comes from the same place. Appended, so -W and pytest still win. +warnings.filterwarnings("always", category=MobiumWarning, append=True) + + @dataclass(frozen=True) class Bounds: """An on-screen rectangle in device pixels, on both platforms.""" @@ -185,13 +195,17 @@ def quit(self) -> None: """End the session on the device and close the connection. The teardown is the daemon's own: accessibility settings put back, a - recording or route stopped, WebViews detached, the device-side server - stopped, and the app start() launched, if any, stopped too. Quitting a + route stopped, WebViews detached, the device-side server stopped, and + the app start() launched, if any, stopped too. A running recording or + audio capture is discarded, with a UserWarning. Quitting a session that is not open succeeds, and a second quit -- say, a with block ending after an explicit one -- does nothing. A script that exits without quit() or close() has the sessions it started ended for it: mobium sees the client go. """ + self._end(stacklevel=3) + + def _end(self, stacklevel: int) -> None: if self._quit: return self._quit = True @@ -199,7 +213,10 @@ def quit(self) -> None: if self.session is not None: args["device"] = self.session.device try: - self._call("app_session", args) + ended = self._data("app_session", args) or {} + for what in ended.get("discarded", []): + warnings.warn(f"{what} was discarded, unsaved: stop it before quit() to keep it", + MobiumWarning, stacklevel=stacklevel) finally: self.close() @@ -215,7 +232,7 @@ def __exit__(self, *_: Any) -> None: # block; one from connect() only closes, leaving the session to # whoever opened it. if self.session is not None: - self.quit() + self._end(stacklevel=3) else: self.close() diff --git a/clients/python/tests/fake_mobium.py b/clients/python/tests/fake_mobium.py index 0ec62a8..fd9e061 100644 --- a/clients/python/tests/fake_mobium.py +++ b/clients/python/tests/fake_mobium.py @@ -73,6 +73,8 @@ def send(obj): "driver": "wda" if platform == "ios" else "uiautomator2", "app": args.get("app", ""), "sessions": []} elif action == "end": view = {"action": "end", "device": args.get("device", ""), "ended": True, "sessions": []} + if os.environ.get("MOBIUM_FAKE") == "discards": + view["discarded"] = ["the screen recording"] else: view = {"action": "status", "sessions": []} send({"jsonrpc": "2.0", "id": i, "result": {"content": [{"type": "text", "text": action or "status"}], diff --git a/clients/python/tests/test_connection.py b/clients/python/tests/test_connection.py index 3a0118d..ced04f2 100644 --- a/clients/python/tests/test_connection.py +++ b/clients/python/tests/test_connection.py @@ -9,6 +9,7 @@ import tempfile import threading import time +import warnings sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) @@ -233,6 +234,33 @@ def one(n): check(raises(MobiumError, d.map) is not None, "a call after quit succeeded") check(connect(binary=FAKE).session is None, "connect reported a session it did not start") +# -- quit warns about what it discarded --------------------------------------- +os.environ["MOBIUM_FAKE"] = "discards" +d = start(platform="android", binary=FAKE) +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + d.quit() +check(len(caught) == 1 and "the screen recording was discarded" in str(caught[0].message), + f"quit did not warn about the recording: {[str(w.message) for w in caught]}") +os.environ["MOBIUM_FAKE"] = "ok" +d = start(platform="android", binary=FAKE) +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + d.quit() +check(not caught, f"quit warned with nothing discarded: {[str(w.message) for w in caught]}") + +# Every time: Python shows a warning once per place unless told otherwise. +os.environ["MOBIUM_FAKE"] = "discards" +shown = [] +keep = warnings.showwarning +warnings.showwarning = lambda message, *a, **k: shown.append(str(message)) +for _ in range(3): + with start(platform="android", binary=FAKE): + pass +warnings.showwarning = keep +check(len(shown) == 3, f"three discards warned {len(shown)} times") +os.environ["MOBIUM_FAKE"] = "ok" + # -- a with block around start() quits on the way out ------------------------ with start(platform="android", binary=FAKE) as d: check(d.session is not None and d.session.driver == "uiautomator2", "start in a with block did not open a session") diff --git a/cmd/mobium/pipe.go b/cmd/mobium/pipe.go index 34f21c5..e435f72 100644 --- a/cmd/mobium/pipe.go +++ b/cmd/mobium/pipe.go @@ -4,6 +4,7 @@ import ( "bufio" "encoding/json" "fmt" + "io" "os" "os/signal" "sort" @@ -101,6 +102,7 @@ type pipe struct { owned map[string]bool // devices whose session this client started detached bool released bool + stderr io.Writer // os.Stderr unless a test sets it } func newPipe(call func(string, map[string]interface{}, map[string]interface{}) (*agent.ToolsCallResult, error)) *pipe { @@ -125,12 +127,40 @@ func (p *pipe) release() { p.mu.Unlock() sort.Strings(devices) for _, d := range devices { - if _, err := p.call("app_session", map[string]interface{}{"action": "end", "device": d}, nil); err != nil { + res, err := p.call("app_session", map[string]interface{}{"action": "end", "device": d}, nil) + for _, what := range discardedBy(res) { + fmt.Fprintf(p.stderrOr(), "mobium: the client went away, and ending its session on %s discarded %s, unsaved\n", d, what) + } + if err != nil { fmt.Fprintf(os.Stderr, "mobium pipe: the client went away and its session on %s could not be ended: %v\n", d, err) } } } +// discardedBy reads what an app_session end threw away. +func discardedBy(result *agent.ToolsCallResult) []string { + if result == nil || result.IsError { + return nil + } + data, err := json.Marshal(result.StructuredContent) + if err != nil { + return nil + } + var view agent.SessionView + if json.Unmarshal(data, &view) != nil { + return nil + } + return view.Discarded +} + +// stderrOr is where the pipe tells the client's person something. +func (p *pipe) stderrOr() io.Writer { + if p.stderr != nil { + return p.stderr + } + return os.Stderr +} + // track follows app_session through a call, so the pipe knows which sessions // are the client's own. func (p *pipe) track(name string, result *agent.ToolsCallResult) { diff --git a/cmd/mobium/pipe_test.go b/cmd/mobium/pipe_test.go index 0718428..18e2c99 100644 --- a/cmd/mobium/pipe_test.go +++ b/cmd/mobium/pipe_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "encoding/json" "strings" "testing" @@ -103,8 +104,9 @@ func TestPipeResponsesAreValidJSON(t *testing.T) { // fakeDaemon answers app_session as the daemon does, and records every call. type fakeDaemon struct { - calls []string - open map[string]bool + calls []string + open map[string]bool + discarded []string } func (f *fakeDaemon) call(tool string, args, _ map[string]interface{}) (*agent.ToolsCallResult, error) { @@ -121,6 +123,7 @@ func (f *fakeDaemon) call(tool string, args, _ map[string]interface{}) (*agent.T f.open[device] = true case "end": view.Ended = f.open[device] + view.Discarded = f.discarded delete(f.open, device) } return agent.Result(action, view), nil @@ -192,3 +195,27 @@ func TestPipeLeavesWhatItDidNotStartOrWasToldToKeep(t *testing.T) { t.Errorf("a detached client's session was ended: %v", got) } } + +// A client that exits without quit never reads the answer, so the pipe says +// what its leaving threw away. CHALLENGES 388. +func TestPipeSaysWhatTheClientsGoingDiscarded(t *testing.T) { + f := &fakeDaemon{open: map[string]bool{}, discarded: []string{"the screen recording"}} + var said bytes.Buffer + p := newPipe(f.call) + p.stderr = &said + p.handle([]byte(startCall("emulator-5554"))) + p.release() + if !strings.Contains(said.String(), "discarded the screen recording, unsaved") { + t.Errorf("said %q", said.String()) + } + + f = &fakeDaemon{open: map[string]bool{}} + said.Reset() + p = newPipe(f.call) + p.stderr = &said + p.handle([]byte(startCall("emulator-5554"))) + p.release() + if said.Len() != 0 { + t.Errorf("said %q with nothing discarded", said.String()) + } +} diff --git a/docs/CHALLENGES.md b/docs/CHALLENGES.md index 3b1d554..575bd06 100644 --- a/docs/CHALLENGES.md +++ b/docs/CHALLENGES.md @@ -7,8 +7,8 @@ the source. The pattern across them is the reason the document exists: **almost nothing here was found by reading code or by a test written from imagination.** Of -387 defects, 310 were found only by running against a real device. Sixteen -more — 255, 280, 282, 327, 329, 353, 355, 356, 359, 371, 375, 380, 382, 384, 386 and 387 — were met while +388 defects, 310 were found only by running against a real device. Seventeen +more — 255, 280, 282, 327, 329, 353, 355, 356, 359, 371, 375, 380, 382, 384, 386, 387 and 388 — were met while running against one, though the fault lay in a check, the host or Mobium's own design rather than in the platform. The other sixty-one — 4, 5, 6, 14, 23, 24, 32, 35, 36, 39, 44, 50, 53, 54, 57, 66, 89, 99, 100, 121, 123, 129, 130, @@ -8523,6 +8523,33 @@ is marked as device text. alone. With the fix, 0 of 203 and 0 of 193. All five clients and MCP took the empty ref on the Android 15 emulator. `wait-states.sh` fails on `main` on both devices and passes with the fix. +### 388. A client's quit discarded a recording without a word + +**Found by:** a person recording a pytest run, 2026-11-04 +(mobiumdev/mobium#328), on the Android 15 emulator and the iOS 26.5 simulator. +283 made `session end` say what it discards, but only in its text. Every client +makes that call and drops the answer, so a recording was lost at exit 0. + +**Fixed:** `app_session` and `app_shutdown` answer `discarded`, and each client's +quit warns: Python and JavaScript a `MobiumWarning`, Go the standard logger, Java +the `dev.mobium` logger, .NET standard error. Python shows a warning once per +place by default, so three lost recordings warned once; it now warns each time, +and `-W ignore` still silences it. A client that exits without quit, `kill -9` +included, has the pipe say it on standard error. The list now names a running +trace and a speech listen too: a trace was lost at session end with no word. + +A recording and a trace leave audio's note (280), so a later status or stop names +the cause instead of "nothing is recording": another client's quit, `daemon stop`, +a driver switch, a shutdown. + +**Measured:** with a running recording, all five clients on the Android 15 +emulator; Python, exit without quit and a trace on the iOS 26.5 simulator. pytest +shows the warning in its summary. A speech listen is covered by a test alone: +TalkBack here does not log at VERBOSE. + +**Still so:** a client's `start()` reuses a session the CLI opened, and its quit +ends it, so `mobium record` cannot film a client's run. It now warns, and the +CLI's stop names why. `mobium test` over a running recording kept it, 1 run. ## Findings that were not defects diff --git a/docs/RELEASE-CHECKLIST.md b/docs/RELEASE-CHECKLIST.md index a8f262c..c608f9c 100644 --- a/docs/RELEASE-CHECKLIST.md +++ b/docs/RELEASE-CHECKLIST.md @@ -4,7 +4,7 @@ [CI workflow](../.github/workflows/ci.yml) runs on every push — formatting, vet, both modules' tests, the four non-Go clients' own tests, and six cross-compile targets. This file is the rest, and the rest is where most -serious defects in this project have come from: 310 of 387 were found only by +serious defects in this project have come from: 310 of 388 were found only by running against a real device. Work through it before tagging a release, on three substrates: an Android diff --git a/internal/agent/audionote.go b/internal/agent/audionote.go index 6544e83..20f7107 100644 --- a/internal/agent/audionote.go +++ b/internal/agent/audionote.go @@ -27,16 +27,26 @@ type audioNote struct { Why string `json:"why,omitempty"` } -func audioNotePath(serial string) string { +// A screen recording and a trace live in the daemon too, and leave the same +// note. CHALLENGES 388. +const ( + audioNoteKind = "audio" + recordingNoteKind = "recording" + traceNoteKind = "trace" +) + +func notePath(kind, serial string) string { name := paths.SessionName() if name == "" { name = "default" } // SessionDir's last segment is the serial made safe for a path. - return filepath.Join(paths.DaemonDir(), "audio-"+name+"-"+filepath.Base(paths.SessionDir(serial))+".json") + return filepath.Join(paths.DaemonDir(), kind+"-"+name+"-"+filepath.Base(paths.SessionDir(serial))+".json") } -func writeAudioNote(serial string, n audioNote) { +func writeAudioNote(serial string, n audioNote) { writeNote(audioNoteKind, serial, n) } + +func writeNote(kind, serial string, n audioNote) { data, err := json.Marshal(n) if err != nil { return @@ -44,15 +54,19 @@ func writeAudioNote(serial string, n audioNote) { if err := os.MkdirAll(paths.DaemonDir(), 0o700); err != nil { return } - _ = os.WriteFile(audioNotePath(serial), data, 0o600) + _ = os.WriteFile(notePath(kind, serial), data, 0o600) } -func removeAudioNote(serial string) { _ = os.Remove(audioNotePath(serial)) } +func removeAudioNote(serial string) { removeNote(audioNoteKind, serial) } + +func removeNote(kind, serial string) { _ = os.Remove(notePath(kind, serial)) } // lostAudio says what became of a capture on this device that this daemon // does not hold, or "" when none was lost. -func lostAudio(serial string) string { - data, err := os.ReadFile(audioNotePath(serial)) +func lostAudio(serial string) string { return lostNote(audioNoteKind, "capture", serial) } + +func lostNote(kind, what, serial string) string { + data, err := os.ReadFile(notePath(kind, serial)) if err != nil { return "" } @@ -68,13 +82,15 @@ func lostAudio(serial string) string { default: why = "its session was closed" } - return fmt.Sprintf("the capture started at %s, %s ago, was lost: %s", n.Started.Format("15:04:05"), + return fmt.Sprintf("the %s started at %s, %s ago, was lost: %s", what, n.Started.Format("15:04:05"), time.Since(n.Started).Round(time.Second), why) } // endAudioNote records why a capture ended other than by a stop. -func endAudioNote(serial, why string) { - data, err := os.ReadFile(audioNotePath(serial)) +func endAudioNote(serial, why string) { endNote(audioNoteKind, serial, why) } + +func endNote(kind, serial, why string) { + data, err := os.ReadFile(notePath(kind, serial)) if err != nil { return } @@ -83,5 +99,5 @@ func endAudioNote(serial, why string) { return } n.Why = why - writeAudioNote(serial, n) + writeNote(kind, serial, n) } diff --git a/internal/agent/backend.go b/internal/agent/backend.go index 55893be..cf1e205 100644 --- a/internal/agent/backend.go +++ b/internal/agent/backend.go @@ -214,6 +214,34 @@ func (s *session) closeWeb() { s.webApp, s.webAppID = "", "" } +// whyClosed says why the session is closing, for what it discards to say. +func (s *session) whyClosed() string { + if s.closeWhy != "" { + return s.closeWhy + } + return "its session was closed" +} + +// discards names what ending the session throws away unsaved. CHALLENGES 283, +// 388. +func (s *session) discards() []string { + var out []string + if s.audio != nil { + out = append(out, fmt.Sprintf("the audio capture running for %s", + time.Since(s.audio.Started()).Round(time.Second))) + } + if s.recording != nil { + out = append(out, "the screen recording") + } + if s.trace != nil { + out = append(out, fmt.Sprintf("the trace running for %s", time.Since(s.trace.started).Round(time.Second))) + } + if s.listening != nil { + out = append(out, "what the screen reader said since speech --listen start") + } + return out +} + // close releases a backend that owns a device-side process. func (s *session) close() { // Finished cleanly and kept nowhere: a recorder that is killed leaves an @@ -224,6 +252,11 @@ func (s *session) close() { s.recording.Discard(ctx) cancel() s.recording = nil + endNote(recordingNoteKind, s.dev.Serial, s.whyClosed()) + } + if s.trace != nil { + s.trace = nil + endNote(traceNoteKind, s.dev.Serial, s.whyClosed()) } if s.listening != nil { s.listening.end() @@ -234,11 +267,7 @@ func (s *session) close() { s.audio.Discard(ctx) cancel() s.audio = nil - why := s.closeWhy - if why == "" { - why = "its session was closed" - } - endAudioNote(s.dev.Serial, why) + endAudioNote(s.dev.Serial, s.whyClosed()) } s.stopRoute() // Before the driver closes: an undo needs the device to reach. diff --git a/internal/agent/boot.go b/internal/agent/boot.go index f42b7f7..259986b 100644 --- a/internal/agent/boot.go +++ b/internal/agent/boot.go @@ -86,6 +86,8 @@ type ShutdownView struct { Device string `json:"device"` // SessionEnded says this daemon had a session on it, ended first. SessionEnded bool `json:"session_ended"` + // Discarded names what ending that session threw away unsaved. + Discarded []string `json:"discarded,omitempty"` } // shutdown ends this daemon's session on the device, then the device: the @@ -110,6 +112,7 @@ func (h *Handlers) shutdown(ctx context.Context, args map[string]interface{}) (* view := ShutdownView{Device: serial} for key, s := range h.sessions { if key == serial || s.dev.Serial == serial { + view.Discarded = append(view.Discarded, s.discards()...) s.closeWhy = "the device was shut down" s.close() delete(h.sessions, key) @@ -125,7 +128,7 @@ func (h *Handlers) shutdown(ctx context.Context, args map[string]interface{}) (* if err != nil { return nil, err } - return Result(fmt.Sprintf("shut down %s", serial), view), nil + return Result(fmt.Sprintf("shut down %s", serial)+discardedText(view.Discarded), view), nil } // shutdownTarget resolves a name to a running emulator's serial or a booted diff --git a/internal/agent/main_test.go b/internal/agent/main_test.go new file mode 100644 index 0000000..ebdb2c5 --- /dev/null +++ b/internal/agent/main_test.go @@ -0,0 +1,19 @@ +package agent + +import ( + "os" + "testing" +) + +// TestMain keeps what the tools write beside the daemon's socket, such as a +// lost capture's note, out of the home of whoever runs the tests. +func TestMain(m *testing.M) { + home, err := os.MkdirTemp("", "mobium-agent-test-") + if err != nil { + panic(err) + } + os.Setenv("MOBIUM_HOME", home) + code := m.Run() + os.RemoveAll(home) + os.Exit(code) +} diff --git a/internal/agent/outputschema_gen.go b/internal/agent/outputschema_gen.go index 3da2378..2d470cd 100644 --- a/internal/agent/outputschema_gen.go +++ b/internal/agent/outputschema_gen.go @@ -57,10 +57,10 @@ var outputSchemas = map[string]string{ "app_rotate": "{\"properties\":{\"degrees\":{\"type\":\"number\"},\"radius\":{\"type\":\"integer\"},\"target\":{\"type\":\"string\"},\"x\":{\"type\":\"integer\"},\"y\":{\"type\":\"integer\"}},\"required\":[\"degrees\",\"radius\",\"x\",\"y\"],\"type\":\"object\"}", "app_screen": "{\"properties\":{\"applied\":{\"type\":\"string\"},\"device\":{\"type\":\"string\"},\"dpi\":{\"type\":\"integer\"},\"findings\":{\"items\":{\"properties\":{\"bounds\":{\"properties\":{\"x1\":{\"type\":\"integer\"},\"x2\":{\"type\":\"integer\"},\"y1\":{\"type\":\"integer\"},\"y2\":{\"type\":\"integer\"}},\"required\":[\"x1\",\"x2\",\"y1\",\"y2\"],\"type\":\"object\"},\"detail\":{\"type\":\"string\"},\"kind\":{\"type\":\"string\"},\"label\":{\"type\":\"string\"},\"locator\":{\"type\":\"string\"},\"path\":{\"type\":\"string\"}},\"required\":[\"bounds\",\"detail\",\"kind\",\"label\"],\"type\":\"object\"},\"type\":[\"array\",\"null\"]},\"height_dp\":{\"type\":\"integer\"},\"height_px\":{\"type\":\"integer\"},\"overridden\":{\"type\":\"boolean\"},\"physical_dpi\":{\"type\":\"integer\"},\"physical_height_px\":{\"type\":\"integer\"},\"physical_width_px\":{\"type\":\"integer\"},\"profiles\":{\"items\":{\"type\":\"string\"},\"type\":[\"array\",\"null\"]},\"smallest_width_dp\":{\"type\":\"integer\"},\"width_dp\":{\"type\":\"integer\"},\"width_px\":{\"type\":\"integer\"}},\"required\":[\"device\",\"height_px\",\"overridden\",\"physical_height_px\",\"physical_width_px\",\"profiles\",\"width_px\"],\"type\":\"object\"}", "app_scroll_to": "{\"properties\":{\"direction\":{\"type\":\"string\"},\"element\":{\"properties\":{\"bounds\":{\"properties\":{\"x1\":{\"type\":\"integer\"},\"x2\":{\"type\":\"integer\"},\"y1\":{\"type\":\"integer\"},\"y2\":{\"type\":\"integer\"}},\"required\":[\"x1\",\"x2\",\"y1\",\"y2\"],\"type\":\"object\"},\"checked\":{\"type\":[\"boolean\",\"null\"]},\"context\":{\"type\":\"string\"},\"covered\":{\"type\":\"string\"},\"disabled\":{\"type\":\"boolean\"},\"focused\":{\"type\":\"boolean\"},\"label\":{\"type\":\"string\"},\"locator\":{\"properties\":{\"exact\":{\"type\":\"boolean\"},\"kind\":{\"type\":\"string\"},\"role\":{\"type\":\"string\"},\"value\":{\"type\":\"string\"}},\"required\":[\"kind\",\"value\"],\"type\":[\"object\",\"null\"]},\"ref\":{\"type\":\"string\"},\"role\":{\"type\":\"string\"},\"selected\":{\"type\":\"boolean\"},\"value\":{\"type\":\"string\"},\"value_scale\":{\"type\":\"string\"}},\"required\":[\"bounds\",\"label\",\"ref\"],\"type\":[\"object\",\"null\"]},\"scrolls\":{\"type\":\"integer\"},\"target\":{\"type\":\"string\"}},\"required\":[\"direction\",\"scrolls\",\"target\"],\"type\":\"object\"}", - "app_session": "{\"properties\":{\"action\":{\"type\":\"string\"},\"app\":{\"type\":\"string\"},\"closed_tabs\":{\"type\":\"integer\"},\"device\":{\"type\":\"string\"},\"driver\":{\"type\":\"string\"},\"ended\":{\"type\":\"boolean\"},\"exposure\":{\"type\":\"string\"},\"platform\":{\"type\":\"string\"},\"reused\":{\"type\":\"boolean\"},\"sessions\":{\"items\":{\"properties\":{\"device\":{\"type\":\"string\"},\"driver\":{\"type\":\"string\"},\"platform\":{\"type\":\"string\"}},\"required\":[\"device\",\"driver\",\"platform\"],\"type\":\"object\"},\"type\":[\"array\",\"null\"]},\"terminated\":{\"type\":\"string\"}},\"required\":[\"action\",\"ended\",\"sessions\"],\"type\":\"object\"}", + "app_session": "{\"properties\":{\"action\":{\"type\":\"string\"},\"app\":{\"type\":\"string\"},\"closed_tabs\":{\"type\":\"integer\"},\"device\":{\"type\":\"string\"},\"discarded\":{\"items\":{\"type\":\"string\"},\"type\":[\"array\",\"null\"]},\"driver\":{\"type\":\"string\"},\"ended\":{\"type\":\"boolean\"},\"exposure\":{\"type\":\"string\"},\"platform\":{\"type\":\"string\"},\"reused\":{\"type\":\"boolean\"},\"sessions\":{\"items\":{\"properties\":{\"device\":{\"type\":\"string\"},\"driver\":{\"type\":\"string\"},\"platform\":{\"type\":\"string\"}},\"required\":[\"device\",\"driver\",\"platform\"],\"type\":\"object\"},\"type\":[\"array\",\"null\"]},\"terminated\":{\"type\":\"string\"}},\"required\":[\"action\",\"ended\",\"sessions\"],\"type\":\"object\"}", "app_set": "{\"properties\":{\"changed\":{\"type\":\"boolean\"},\"checked\":{\"type\":\"boolean\"},\"role\":{\"type\":\"string\"},\"target\":{\"type\":\"string\"},\"wanted\":{\"type\":\"boolean\"}},\"required\":[\"changed\",\"checked\",\"target\",\"wanted\"],\"type\":\"object\"}", "app_shake": "{\"properties\":{\"device\":{\"type\":\"string\"},\"sent\":{\"type\":\"string\"}},\"required\":[\"device\",\"sent\"],\"type\":\"object\"}", - "app_shutdown": "{\"properties\":{\"device\":{\"type\":\"string\"},\"session_ended\":{\"type\":\"boolean\"}},\"required\":[\"device\",\"session_ended\"],\"type\":\"object\"}", + "app_shutdown": "{\"properties\":{\"device\":{\"type\":\"string\"},\"discarded\":{\"items\":{\"type\":\"string\"},\"type\":[\"array\",\"null\"]},\"session_ended\":{\"type\":\"boolean\"}},\"required\":[\"device\",\"session_ended\"],\"type\":\"object\"}", "app_sms": "{\"properties\":{\"device\":{\"type\":\"string\"},\"from\":{\"type\":\"string\"},\"text\":{\"type\":\"string\"}},\"required\":[\"device\",\"from\",\"text\"],\"type\":\"object\"}", "app_source": "{\"properties\":{\"context\":{\"type\":\"string\"},\"device\":{\"type\":\"string\"},\"format\":{\"type\":\"string\"},\"redacted\":{\"type\":\"integer\"},\"scale\":{\"type\":\"number\"},\"source\":{\"type\":\"string\"},\"units\":{\"type\":\"string\"}},\"required\":[\"device\",\"format\",\"redacted\",\"source\"],\"type\":\"object\"}", "app_state": "{\"properties\":{\"app\":{\"type\":\"string\"},\"covered_by\":{\"type\":\"string\"},\"device\":{\"type\":\"string\"},\"state\":{\"type\":\"string\"},\"suspended\":{\"type\":[\"boolean\",\"null\"]}},\"required\":[\"app\",\"device\",\"state\"],\"type\":\"object\"}", diff --git a/internal/agent/record.go b/internal/agent/record.go index 7710228..6d16fb8 100644 --- a/internal/agent/record.go +++ b/internal/agent/record.go @@ -36,6 +36,9 @@ func (h *Handlers) recordOn(ctx context.Context, s *session, args map[string]int switch action := stringArg(args, "action"); action { case "", "status": if s.recording == nil { + if lost := lostNote(recordingNoteKind, "recording", s.dev.Serial); lost != "" { + return Result("not recording — "+lost, view), nil + } return Result("not recording", view), nil } view.Recording = true @@ -56,11 +59,17 @@ func (h *Handlers) recordOn(ctx context.Context, s *session, args map[string]int return nil, err } s.recording = r + writeNote(recordingNoteKind, s.dev.Serial, audioNote{PID: os.Getpid(), Started: r.Started()}) view.Recording = true return Result("recording — stop it with action \"stop\" and a path", view), nil case "stop": if s.recording == nil { + if lost := lostNote(recordingNoteKind, "recording", s.dev.Serial); lost != "" { + removeNote(recordingNoteKind, s.dev.Serial) + return nil, mobiumerr.New(mobiumerr.Internal, "%s", lost).WithRemedy("a recording lives in the " + + "session that started it: start a new one, and stop it before the session or the daemon ends") + } return nil, mobiumerr.New(mobiumerr.InvalidArgument, "nothing is recording — start with action \"start\"") } path := stringArg(args, "path") @@ -88,6 +97,7 @@ func (h *Handlers) recordOn(ctx context.Context, s *session, args map[string]int } r := s.recording s.recording = nil + removeNote(recordingNoteKind, s.dev.Serial) view.Elapsed = time.Since(r.Started()).Round(time.Millisecond) info, err := r.Stop(ctx, path) if err != nil { diff --git a/internal/agent/record_test.go b/internal/agent/record_test.go index de4ef4c..616431d 100644 --- a/internal/agent/record_test.go +++ b/internal/agent/record_test.go @@ -103,3 +103,49 @@ func TestRecordMakesTheFolder(t *testing.T) { t.Fatalf("the folder was not made: %v", err) } } + +// A stop after the session went says what became of the recording, and says it +// once. CHALLENGES 388. +func TestARecordingLostWithItsSessionIsNamed(t *testing.T) { + t.Setenv("MOBIUM_HOME", t.TempDir()) + call, s := withRecorder(t, &fakeRecording{}) + if _, err := call(map[string]interface{}{"action": "start"}); err != nil { + t.Fatal(err) + } + s.closeWhy = "the daemon was stopped" + s.close() + res, err := call(map[string]interface{}{}) + if err != nil || !strings.Contains(textOf(res), "was lost: the daemon was stopped") { + t.Errorf("status: %v, %q", err, textOf(res)) + } + _, err = call(map[string]interface{}{"action": "stop", "path": "/tmp/x.mp4"}) + if err == nil || !strings.Contains(err.Error(), "was lost: the daemon was stopped") { + t.Errorf("stop: %v", err) + } + _, err = call(map[string]interface{}{"action": "stop", "path": "/tmp/x.mp4"}) + if err == nil || !strings.Contains(err.Error(), "nothing is recording") { + t.Errorf("a second stop: %v", err) + } +} + +// A stop refused for its missing path leaves the recording, so nothing may say +// it was lost. CHALLENGES 388. +func TestARecordingStoppedCleanlyLeavesNoNote(t *testing.T) { + t.Setenv("MOBIUM_HOME", t.TempDir()) + call, s := withRecorder(t, &fakeRecording{info: device.MP4Info{Frames: 1, Duration: time.Second}}) + if _, err := call(map[string]interface{}{"action": "start"}); err != nil { + t.Fatal(err) + } + if _, err := call(map[string]interface{}{"action": "stop"}); err == nil { + t.Fatal("a stop with no path was accepted") + } + if lost := lostNote(recordingNoteKind, "recording", s.dev.Serial); lost == "" { + t.Error("a running recording left no note") + } + if _, err := call(map[string]interface{}{"action": "stop", "path": t.TempDir() + "/x.mp4"}); err != nil { + t.Fatal(err) + } + if lost := lostNote(recordingNoteKind, "recording", s.dev.Serial); lost != "" { + t.Errorf("a saved recording reads as %q", lost) + } +} diff --git a/internal/agent/session.go b/internal/agent/session.go index 2254eb3..ef9c5bc 100644 --- a/internal/agent/session.go +++ b/internal/agent/session.go @@ -32,6 +32,9 @@ type SessionView struct { // open, where something can: a real Android phone's UiAutomator2 server // listens on the phone's network as well as adb (CHALLENGES 154). Exposure string `json:"exposure,omitempty"` + // Discarded names what end threw away unsaved: a screen recording, an + // audio capture. CHALLENGES 388. + Discarded []string `json:"discarded,omitempty"` // Ended says end closed a session; false means there was none to close. Ended bool `json:"ended"` Sessions []SessionInfo `json:"sessions"` @@ -201,14 +204,8 @@ func (h *Handlers) sessionEnd(args map[string]interface{}) (*ToolsCallResult, er restored := h.restoreNetwork(s) // What the end discards, said with it: the capture went without a word, // and only a later status told anyone. CHALLENGES 283. - var discarded []string - if s.audio != nil { - discarded = append(discarded, fmt.Sprintf("the audio capture running for %s", - time.Since(s.audio.Started()).Round(time.Second))) - } - if s.recording != nil { - discarded = append(discarded, "the screen recording") - } + discarded := s.discards() + view.Discarded = discarded s.closeWhy = "the session was ended" s.close() delete(h.sessions, keys[0]) @@ -223,13 +220,7 @@ func (h *Handlers) sessionEnd(args map[string]interface{}) (*ToolsCallResult, er if restored { text += "; the network is as it was before app_network changed it" } - if len(discarded) > 0 { - verb, it := " was", "it" - if len(discarded) > 1 { - verb, it = " were", "them" - } - text += "; " + strings.Join(discarded, " and ") + verb + " discarded, unsaved — stop " + it + " first to keep " + it - } + text += discardedText(discarded) switch { case stopped == nil && s.launched != "": view.Terminated = s.launched @@ -241,6 +232,19 @@ func (h *Handlers) sessionEnd(args map[string]interface{}) (*ToolsCallResult, er return Result(text, view), nil } +// discardedText is what a tool that ended a session says about discarded, or +// "" when there is nothing to say. +func discardedText(discarded []string) string { + if len(discarded) == 0 { + return "" + } + verb, it := " was", "it" + if len(discarded) > 1 { + verb, it = " were", "them" + } + return "; " + strings.Join(discarded, " and ") + verb + " discarded, unsaved — stop " + it + " first to keep " + it +} + // endTerminateTimeout bounds stopping the launched app on the way out, so an // unresponsive device cannot hold up the end of a session. Half the daemon's // budget for closing every session at shutdown, so the rest of the teardown diff --git a/internal/agent/session_test.go b/internal/agent/session_test.go index 20ec018..c02abf1 100644 --- a/internal/agent/session_test.go +++ b/internal/agent/session_test.go @@ -345,10 +345,16 @@ func TestEndingSaysWhatItDiscarded(t *testing.T) { if !strings.Contains(text, "the audio capture running for") || !strings.Contains(text, "was discarded, unsaved — stop it first") { t.Errorf("end said %q", text) } + if got := res.StructuredContent.(SessionView).Discarded; len(got) != 1 || !strings.HasPrefix(got[0], "the audio capture running for") { + t.Errorf("discarded = %q", got) + } if got := lostAudio("emulator-5554"); !strings.HasSuffix(got, "the session was ended") { t.Errorf("a later status: %q", got) } res, _ = h.sessionTool(context.Background(), map[string]interface{}{"action": "end", "device": "emulator-5556"}) + if got := res.StructuredContent.(SessionView).Discarded; got != nil { + t.Errorf("a session holding nothing discarded %q", got) + } if strings.Contains(res.Content[0].Text, "discarded") { t.Errorf("a session holding nothing said it discarded something: %q", res.Content[0].Text) } @@ -368,3 +374,26 @@ func TestNotAnsweringADBNamesTheDevice(t *testing.T) { } } } + +// CHALLENGES 388. +func TestEndingASessionNamesEverythingItDiscards(t *testing.T) { + t.Setenv("MOBIUM_HOME", t.TempDir()) + h := twoSessions() + s := h.sessions["emulator-5554"] + s.recording = &fakeRecording{} + s.trace = &sessionTrace{started: time.Now()} + s.listening = &speechListen{} + res, err := h.sessionTool(context.Background(), map[string]interface{}{"action": "end", "device": "emulator-5554"}) + if err != nil { + t.Fatal(err) + } + got := strings.Join(res.StructuredContent.(SessionView).Discarded, "|") + for _, want := range []string{"the screen recording", "the trace running for", "what the screen reader said"} { + if !strings.Contains(got, want) { + t.Errorf("discarded %q, want %q", got, want) + } + } + if text := res.Content[0].Text; !strings.Contains(text, "were discarded, unsaved") { + t.Errorf("end said %q", text) + } +} diff --git a/internal/agent/tracing.go b/internal/agent/tracing.go index b05a8e2..ed11e8a 100644 --- a/internal/agent/tracing.go +++ b/internal/agent/tracing.go @@ -63,6 +63,9 @@ func (h *Handlers) traceTool(ctx context.Context, args map[string]interface{}) ( switch action := stringArg(args, "action"); action { case "", "status": if s.trace == nil { + if lost := lostNote(traceNoteKind, "trace", s.dev.Serial); lost != "" { + return Result("not tracing — "+lost, view), nil + } return Result("not tracing", view), nil } view.Tracing, view.Calls = true, s.trace.rec.Calls() @@ -84,11 +87,17 @@ func (h *Handlers) traceTool(ctx context.Context, args map[string]interface{}) ( t.rec = trace.New(trace.Options{Name: stringArg(args, "name"), Device: s.dev.Serial, Platform: platformOf(s), Version: Version}) s.trace = t + writeNote(traceNoteKind, s.dev.Serial, audioNote{PID: os.Getpid(), Started: t.started}) view.Tracing = true return Result("tracing: every call on "+s.dev.Serial+" is recorded until app_trace stop", view), nil case "stop": if s.trace == nil { + if lost := lostNote(traceNoteKind, "trace", s.dev.Serial); lost != "" { + removeNote(traceNoteKind, s.dev.Serial) + return nil, mobiumerr.New(mobiumerr.Internal, "%s", lost).WithRemedy("a trace lives in the daemon " + + "that started it: start a new one, and stop it before the session or the daemon ends") + } return nil, mobiumerr.New(mobiumerr.InvalidArgument, "no trace is running on %s — start one with "+ "app_trace start", s.dev.Serial) } @@ -99,6 +108,7 @@ func (h *Handlers) traceTool(ctx context.Context, args map[string]interface{}) ( } t := s.trace s.trace = nil + removeNote(traceNoteKind, s.dev.Serial) raw, err := t.rec.Zip() if err != nil { return nil, mobiumerr.New(mobiumerr.DeviceServer, "writing the trace: %w", err)