diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2f85668..c1aec1cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,33 @@ jobs: exit 1 fi + unit_tests: + name: Unit tests on ${{ matrix.os }} + # Tags only publish; tests run on branches/PRs before the tag is cut. + if: ${{ !startsWith(github.ref, 'refs/tags/') }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-24.04, windows-2025-vs2026] + steps: + - name: Checkout repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Setup Flutter + uses: kuhnroyal/flutter-fvm-config-action/setup@c378498f1d1962d33039c3989411093ef8a17b2c # v3.3 + with: + path: '.fvmrc' + cache: true + + - name: Run unit tests + working-directory: src/serious_python + run: | + flutter pub get + flutter test test/ + bridge_example_macos: name: Test Bridge example on macOS (${{ matrix.build_system }}, Python ${{ matrix.python_version }}) if: ${{ !startsWith(github.ref, 'refs/tags/') }} diff --git a/src/serious_python/CHANGELOG.md b/src/serious_python/CHANGELOG.md index deb51f48..64af9b2d 100644 --- a/src/serious_python/CHANGELOG.md +++ b/src/serious_python/CHANGELOG.md @@ -1,6 +1,7 @@ ## 6.1.3 * Fix the `package` command hiding why a step failed when the error went to stdout, such as the `SyntaxError` from compiling an app file, which made `flet build` fail with no explanation unless run with `-vv` ([#262](https://github.com/flet-dev/serious-python/pull/262), [flet-dev/flet#7029](https://github.com/flet-dev/flet/issues/7029)). +* Fix `flet build` hanging at "Packaging Python app..." on Windows when pip or another command it runs writes more than about 1 KB of warnings or errors or waits for input, and failing when a command prints text that isn't UTF-8 ([#263](https://github.com/flet-dev/serious-python/pull/263), [flet-dev/flet#5507](https://github.com/flet-dev/flet/issues/5507), [flet-dev/flet#5013](https://github.com/flet-dev/flet/issues/5013)). ## 6.1.2 diff --git a/src/serious_python/bin/package_command.dart b/src/serious_python/bin/package_command.dart index c5ec4b70..ac724eb5 100644 --- a/src/serious_python/bin/package_command.dart +++ b/src/serious_python/bin/package_command.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -665,29 +666,74 @@ class PackageCommand extends Command { return emptyDir; } + /// Runs a command and returns its exit status. If the command fails, exits + /// with status 1 instead; `exit()` doesn't wait for pending writes, so stderr + /// is flushed first. + /// + /// Both pipes are read while the command runs: once a pipe nobody reads is + /// full, the command blocks writing to it and never exits. On Windows a pipe + /// holds about 1 KB, which a few pip warnings are enough to fill. + /// + /// Stdin is closed, so a command that asks for input, such as pip prompting + /// for index credentials, fails instead of waiting forever. + /// + /// Without --verbose, the output of both pipes is kept, in the order it + /// arrives, so it can be shown if the command fails. Some commands (e.g., + /// compileall) report their errors on stdout. Bytes that aren't UTF-8 are + /// replaced, since Windows tools often write in the console's code page. + /// + /// A process the command started (a compiler's helper server, say) can keep + /// the pipes open after the command exits, so the rest of the output is only + /// waited for briefly. Future runExec(String execPath, List args, {Map? environment}) async { final proc = await Process.start(execPath, args, environment: environment); - // Without --verbose, stdout is kept so it can be shown if a command that - // reports its errors there (e.g., compileall) fails. - final stdoutBuffer = StringBuffer(); - await for (final chunk in proc.stdout.transform(utf8.decoder)) { - if (_verbose) { - verbose(chunk.trim()); - } else { - stdoutBuffer.write(chunk); - } + await proc.stdin.close(); + + final output = StringBuffer(); + final subscriptions = >[]; + final drained = >[]; + void drain(Stream> stream, void Function(String) onVerbose) { + final done = Completer(); + subscriptions.add(stream + .transform(const Utf8Decoder(allowMalformed: true)) + .listen( + (chunk) { + if (_verbose) { + onVerbose(chunk); + } else { + output.write(chunk); + } + }, + onDone: done.complete, + onError: (_) => done.complete(), + cancelOnError: true, + )); + drained.add(done.future); } - if (await proc.exitCode != 0) { - stderr.write(stdoutBuffer.toString()); - stderr.write(await proc.stderr.transform(utf8.decoder).join()); - // exit() doesn't wait for pending writes to stderr. + drain(proc.stdout, (chunk) => verbose(chunk.trim())); + drain(proc.stderr, stderr.write); + + final status = await proc.exitCode; + + await Future.wait(drained).timeout(const Duration(seconds: 5), + onTimeout: () async { + for (final subscription in subscriptions) { + await subscription.cancel(); + } + return const []; + }); + + if (status != 0) { + if (!_verbose) { + stderr.write(output.toString()); + } await stderr.flush(); exit(1); } - return proc.exitCode; + return status; } static bool _isFalsy(String? v) => diff --git a/src/serious_python/test/fixtures/run_exec_harness.dart b/src/serious_python/test/fixtures/run_exec_harness.dart new file mode 100644 index 00000000..4f5ebc85 --- /dev/null +++ b/src/serious_python/test/fixtures/run_exec_harness.dart @@ -0,0 +1,11 @@ +import 'dart:io'; + +import '../../bin/package_command.dart'; + +/// Runs the command given as arguments through `PackageCommand.runExec` and +/// prints the status it returns. `runExec` exits the process when the command +/// fails, so tests run this program in its own process. +Future main(List args) async { + final status = await PackageCommand().runExec(args.first, args.sublist(1)); + stdout.writeln("runExec returned $status"); +} diff --git a/src/serious_python/test/glob_test.dart b/src/serious_python/test/glob_test.dart index a9c47aee..976cef96 100644 --- a/src/serious_python/test/glob_test.dart +++ b/src/serious_python/test/glob_test.dart @@ -20,7 +20,6 @@ void main() { expect(pyCache.matches("a/__pycache"), false); final pyCache2 = Glob("**/__pycache__"); - expect(pyCache2.matches("c:/aaa/bbb/__pycache__"), true); expect(pyCache2.matches("a/b/__pycache__"), true); final binPyCache = Glob("**/bin/__pycache__"); diff --git a/src/serious_python/test/run_exec_test.dart b/src/serious_python/test/run_exec_test.dart new file mode 100644 index 00000000..0e255bb1 --- /dev/null +++ b/src/serious_python/test/run_exec_test.dart @@ -0,0 +1,97 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as path; + +/// Python ships on every GitHub-hosted runner, as `python` on Windows. +final python = Platform.isWindows ? "python" : "python3"; + +/// Longer than any case needs; a command that blocks on a full pipe never +/// finishes, so it fails here instead of hanging the test run. +const hangTimeout = Duration(seconds: 30); + +late Directory tempDir; +late String harness; + +/// Runs [script] with Python through `runExec`, inside the compiled harness. +Future runExec(String script) async { + final proc = await Process.start(harness, [python, "-c", script]); + final stdoutText = proc.stdout.transform(systemEncoding.decoder).join(); + final stderrText = proc.stderr.transform(systemEncoding.decoder).join(); + final status = await proc.exitCode.timeout(hangTimeout, onTimeout: () { + proc.kill(ProcessSignal.sigkill); + fail("runExec didn't return within $hangTimeout"); + }); + return ProcessResult(proc.pid, status, await stdoutText, await stderrText); +} + +void main() { + setUpAll(() async { + tempDir = await Directory.systemTemp.createTemp("run_exec_test"); + harness = + path.join(tempDir.path, Platform.isWindows ? "harness.exe" : "harness"); + final compile = await Process.run( + "dart", + [ + "compile", + "exe", + "test/fixtures/run_exec_harness.dart", + "-o", + harness, + ], + runInShell: true); + expect(compile.exitCode, 0, reason: "${compile.stdout}${compile.stderr}"); + }); + + tearDownAll(() async { + await tempDir.delete(recursive: true); + }); + + // 200 KB is more than a pipe holds on any OS (about 1 KB on Windows, + // 64 KB on macOS and Linux). + + test("a command writing a lot to stderr finishes, and its output is hidden", + () async { + final result = await runExec( + "import sys; sys.stderr.write('w' * 200000); print('done')"); + + expect(result.exitCode, 0); + expect(result.stdout, contains("runExec returned 0")); + expect(result.stderr, isEmpty); + }); + + test("a failing command's stdout and stderr are shown", () async { + final result = await runExec("import sys; print('OUT-MARK'); " + "sys.stderr.write('e' * 200000 + 'ERR-MARK'); sys.exit(3)"); + + expect(result.exitCode, 1); + expect(result.stderr, contains("OUT-MARK")); + expect(result.stderr, contains("ERR-MARK")); + }); + + test("a command reading stdin gets end of input", () async { + final result = await runExec( + "import sys; data = sys.stdin.read(); sys.exit(0 if data == '' else 1)"); + + expect(result.exitCode, 0); + expect(result.stdout, contains("runExec returned 0")); + }); + + test("output that isn't UTF-8 is shown", () async { + final result = await runExec("import sys; " + "sys.stdout.buffer.write(b'caf\\xe9 BAD-BYTES'); sys.exit(2)"); + + expect(result.exitCode, 1); + expect(result.stderr, contains("BAD-BYTES")); + }); + + test("a process left running with the pipes open doesn't block", () async { + // The process outlives hangTimeout, so waiting for its pipes to close + // fails the test. + final result = await runExec("import subprocess, sys; " + "subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)'])"); + + expect(result.exitCode, 0); + expect(result.stdout, contains("runExec returned 0")); + }); +}