Repository navigation
[native_toolchain_c] Use ProcessManager and FileSystem #3072
Description
Most of the time on our CI is spent running integration tests for package:native_toolchain_c and package:hooks_runner. (#90 (comment))
While it is valuable to run integration tests on CI, it would be nice if locally we could distinguish between integration tests and unit tests.
We could write unit tests in a style like flutter tools where we mock the file system and process calls.
This is quite a lot of tedious work though.
Activity
Varadraj75 commentedon Aug 7, 2026
Hi @dcharkes, is this one still up for grabs? I'd like to take it.
Had a look at the package first. Process spawning already goes through
runProcessinsrc/utils/run_process.dart(plus oneProcess.runinenv_from_bat.dart), so wiring aProcessManagerin there is small. The real work is theFile/Directoryusage, spread across about a dozen files (cbuilder, compiler_resolver, tool_resolver, the toolchain resolvers).My plan: add
package:fileandpackage:process, default toLocalFileSystemandLocalProcessManagerso behavior stays the same, then add flutter_tools style unit tests withMemoryFileSystemand a fake process manager that just check the compiler and linker invocations without running real toolchains. Since this is a lot of tedious work I'd rather do it in small PRs, the process seam first, then file system area by area.Two questions before I start:
fileSystemandprocessManagerbe exposed on the publicCBuilder/CLinkerconstructors or kept internal?package:processwould be a new dependency here (hooks_runner already usespackage:file). Any concern since these packages roll into the SDK?dcharkes commentedon Aug 7, 2026
I think they should be exposed as optional params in the same place as
Logger.Not really a concern. Major version bumps could be a bit tricky, but many other packages are depending on it too already. We don't really use native_toolchain_c directly in Flutter and Dart SDK anyway, only in hooks. So actual breaking changes to the public API, if they were to happen, affect only the integration test projects in the Flutter and Dart SDK.
Yes! Still up for grabs. 🚀
Varadraj75 commentedon Aug 8, 2026
Thanks! Starting with the process manager part then.
[native_toolchain_c] Inject ProcessManager for process spawning
[native_toolchain_c] Fix tool lookup on Windows, rework test fake
Varadraj75 commentedon Aug 13, 2026
Next up, the file system half. I mapped the dart:io usage and I'd split it the same way as the process manager work:
Public APIs all take Uri, so nothing user-facing changes. Starting with 1 unless you'd rather see it sliced differently. Should the integration/unit test directory reorganization come after these, or fold into them?
dcharkes commentedon Aug 13, 2026
Exactly, that's on purpose! 👌
So you are suggesting to split it up in multiple PRs and only have the final PR add the public API
fileSystemparameter. That could work. But I'm also fine with a PR that combines everything into 1 if that's easier. Maybe that's easier to verify: you can write a unit test that mocks both the process manager and the file system and expect a bunch of process calls and file reads to see that it all works as expected.(And then we can follow up after that to separate the test suite in unit tests and integration tests, so that we get a lot of test coverage fast for the unit tests already.)
Varadraj75 commentedon Aug 14, 2026
Combined it is, that verification argument sold me. One PR: FileSystem on ToolResolvingContext plus a fileSystem param on the builders mirroring processManager, all dart:io file usage converted, and a unit test that fakes both seams and asserts the process invocations and file operations for a CBuilder run. Test suite split as the follow-up.