From 02a8ba9c1812fc7285b34070213f1cf9625c546e Mon Sep 17 00:00:00 2001 From: Mikkeli Date: Wed, 26 Aug 2026 19:03:59 +0900 Subject: [PATCH] fix: report a user-requested stop as stopped rather than failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stopping a test shuts down the control socket, so iperf_run_client() returns -1 with "unable to send control message ... Broken pipe". That is the expected consequence of the stop, not a fault, but the bridge read the negative return as a failure — so pressing STOP left the UI showing "Error (exit 1)" above a broken-pipe message. The bridge now consults its own g_stop flag before classifying the outcome, and the distinction travels up as Iperf3Result.stopped so the status line can say "Stopped". A stop is reported with exit code 0: the run ended the way the user asked it to. Co-Authored-By: Claude Opus 5 --- android/app/src/main/cpp/iperf_jni.c | 8 +++++- lib/data/services/iperf3_service.dart | 10 +++++++- lib/domain/models/iperf3_result.dart | 5 ++++ lib/ui/features/test_run/test_run_screen.dart | 5 +++- .../test_run/test_run_view_model.dart | 3 +++ test/domain/models/iperf3_result_test.dart | 10 ++++++++ .../test_run/test_run_view_model_test.dart | 25 +++++++++++++++++++ 7 files changed, 63 insertions(+), 3 deletions(-) diff --git a/android/app/src/main/cpp/iperf_jni.c b/android/app/src/main/cpp/iperf_jni.c index cf4f9ea..5b4cfac 100644 --- a/android/app/src/main/cpp/iperf_jni.c +++ b/android/app/src/main/cpp/iperf_jni.c @@ -251,8 +251,14 @@ static void iperf_cleanup_and_signal(JNIEnv *env, retire_test(); free_argv(argv, argc); + /* A user-requested stop shuts down the control socket, so + * iperf_run_client() returns -1 with "Broken pipe" — expected, not a + * failure. Check g_stop so that path is reported as a stop, not an error. */ + int user_stopped = stopped || g_stop; + g_stop = 0; + /* Signal result to Java */ - if (stopped) { + if (user_stopped) { signal_output(env, state, "[iPerf JNI] Test was stopped by user.\n"); } else if (ret < 0) { char errbuf[256]; diff --git a/lib/data/services/iperf3_service.dart b/lib/data/services/iperf3_service.dart index 24e9e43..a3e6296 100644 --- a/lib/data/services/iperf3_service.dart +++ b/lib/data/services/iperf3_service.dart @@ -54,9 +54,13 @@ class Iperf3Service { /// Whether the native layer reported an error during this run. bool _sawError = false; + /// Whether [stop] was called for the current run. + bool _stopRequested = false; + /// Cancel any running test. Future stop() async { if (!_isRunning) return; + _stopRequested = true; try { await _channel.invokeMethod('stopIperf'); } catch (e) { @@ -83,6 +87,7 @@ class Iperf3Service { _isRunning = true; _sawError = false; + _stopRequested = false; _outputBuffer.clear(); final args = config.toCliArgs(); @@ -157,7 +162,10 @@ class Iperf3Service { completer.complete( Iperf3Result( rawOutput: _transcript(), - exitCode: _sawError ? exitCodeNativeError : 0, + // A user-requested stop is a normal outcome, not a failure. + exitCode: + (_sawError && !_stopRequested) ? exitCodeNativeError : 0, + stopped: _stopRequested, ), ); } diff --git a/lib/domain/models/iperf3_result.dart b/lib/domain/models/iperf3_result.dart index 2c54feb..ef1b990 100644 --- a/lib/domain/models/iperf3_result.dart +++ b/lib/domain/models/iperf3_result.dart @@ -3,8 +3,13 @@ class Iperf3Result { final String rawOutput; final int exitCode; + /// Whether the run ended because the user asked it to stop, rather than + /// finishing its configured duration or failing. + final bool stopped; + const Iperf3Result({ required this.rawOutput, required this.exitCode, + this.stopped = false, }); } diff --git a/lib/ui/features/test_run/test_run_screen.dart b/lib/ui/features/test_run/test_run_screen.dart index 56285ca..fb2a85d 100644 --- a/lib/ui/features/test_run/test_run_screen.dart +++ b/lib/ui/features/test_run/test_run_screen.dart @@ -166,7 +166,10 @@ class _TestRunContentState extends State<_TestRunContent> { final (String label, Color color) = switch (vm.state) { TestRunState.idle => ('Ready', scheme.onSurfaceVariant), TestRunState.running => ('Running…', scheme.primary), - TestRunState.completed => ('Test complete', scheme.primary), + TestRunState.completed => ( + vm.wasStopped ? 'Stopped' : 'Test complete', + vm.wasStopped ? scheme.onSurfaceVariant : scheme.primary, + ), TestRunState.error => ( 'Error${vm.result != null ? ' (exit ${vm.result!.exitCode})' : ''}', scheme.error, diff --git a/lib/ui/features/test_run/test_run_view_model.dart b/lib/ui/features/test_run/test_run_view_model.dart index d247b10..ec267ec 100644 --- a/lib/ui/features/test_run/test_run_view_model.dart +++ b/lib/ui/features/test_run/test_run_view_model.dart @@ -39,6 +39,9 @@ class TestRunViewModel extends ChangeNotifier { /// Whether a test is currently in flight. bool get isRunning => _state == TestRunState.running; + /// Whether the most recent finished run ended because the user stopped it. + bool get wasStopped => _result?.stopped ?? false; + /// Start a test with the given configuration. Future runTest(TestConfiguration config) async { if (_state == TestRunState.running) return; diff --git a/test/domain/models/iperf3_result_test.dart b/test/domain/models/iperf3_result_test.dart index 68567d7..89070e7 100644 --- a/test/domain/models/iperf3_result_test.dart +++ b/test/domain/models/iperf3_result_test.dart @@ -11,5 +11,15 @@ void main() { expect(result.rawOutput, equals('Test complete')); expect(result.exitCode, equals(0)); }); + + test('stopped defaults to false', () { + const result = Iperf3Result(rawOutput: 'x', exitCode: 0); + expect(result.stopped, isFalse); + }); + + test('stopped is preserved when set', () { + const result = Iperf3Result(rawOutput: 'x', exitCode: 0, stopped: true); + expect(result.stopped, isTrue); + }); }); } diff --git a/test/ui/features/test_run/test_run_view_model_test.dart b/test/ui/features/test_run/test_run_view_model_test.dart index 77088c3..6f83d36 100644 --- a/test/ui/features/test_run/test_run_view_model_test.dart +++ b/test/ui/features/test_run/test_run_view_model_test.dart @@ -120,6 +120,31 @@ void main() { await run; }); + test('wasStopped reflects a user-requested stop', () async { + final mock = _MockIperf3Service( + onRun: (_) async => const Iperf3Result( + rawOutput: 'stopped early', + exitCode: 0, + stopped: true, + ), + ); + final vm = TestRunViewModel(iperf3Service: mock); + await vm.runTest(const TestConfiguration(server: '192.168.1.1')); + // A user stop is a normal outcome: completed, not error. + expect(vm.state, equals(TestRunState.completed)); + expect(vm.wasStopped, isTrue); + }); + + test('wasStopped is false for a run that finished on its own', () async { + final mock = _MockIperf3Service( + onRun: (_) async => + const Iperf3Result(rawOutput: 'done', exitCode: 0), + ); + final vm = TestRunViewModel(iperf3Service: mock); + await vm.runTest(const TestConfiguration(server: '192.168.1.1')); + expect(vm.wasStopped, isFalse); + }); + test('a second runTest while running is ignored', () async { final mock = _MockIperf3Service(); final gate = Completer();