feat: add exitOverride parameter for testing#8
Conversation
Adds an optional `exitOverride` function parameter to `unixSingleInstance` to intercept calls to `exit()`. This allows testing scenarios that normally terminate the process without actually killing the test runner. When provided, the override is called with the intended exit code, and the function returns `false`. Also adds a test case to verify `exitOverride` is correctly triggered when a second instance tries to start.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
@jules we may also want to overload (probably by way of interface) without breaking the existing users of the interface a way to overwrite the socket component for the purpose of logic testing, simulated sockets. We should also ensure that the socket code is tested against a real machine but that can be done once it's decoupled.
There was a problem hiding this comment.
I've abstracted the socket component into a new SocketProvider interface with a DefaultSocketProvider that falls back to the original dart:io logic. I've also updated unixSingleInstance to accept an optional SocketProvider parameter. The tests are still passing and running against a real socket on the filesystem to verify that the decoupling works properly.
Introduces a `SocketProvider` interface to decouple the socket logic from `dart:io`. This allows test runners to mock or simulate sockets without relying on a real filesystem. `unixSingleInstance` now accepts an optional `socketProvider` parameter which defaults to `DefaultSocketProvider()`.
There was a problem hiding this comment.
I've added a test that specifically utilizes the new SocketProvider injection point by providing a MockSocketProvider. It verifies that unixSingleInstance correctly attempts to bind via the injected provider rather than the default dart:io implementation.
Adds a `MockSocketProvider` in the test suite to verify that a custom `SocketProvider` can be correctly passed to `unixSingleInstance`. The test proves the dependency is utilized instead of defaulting to `dart:io` implementations.
There was a problem hiding this comment.
@jules we can have a test or two testing the real world unix socket too if we don't already. Do what is necessary to increase assurance as well as coverage
There was a problem hiding this comment.
The first two existing tests in the suite (First instance returns true and Second instance returns false and sends args to first instance) already use the default socket provider to create a real world UNIX socket in the system's temporary directory. To increase coverage and assurance, I've added a new test (Dead socket is deleted and first instance starts) that verifies the system can recover correctly when it encounters a stale socket file from a previously crashed instance.
|
@jules is there a chance that the interface is missing the clean up logic |
Adds a new test "Dead socket is deleted and first instance starts" to simulate the cleanup of a stale socket file from a previously crashed process. This uses the default real-world socket implementation, increasing coverage on the file deletion path.
Good catch! I've updated the |
Adds `checkSocketExists` and `clearSocket` to the `SocketProvider` interface to fully decouple the dependency on `dart:io` `File` operations. Updates `unixSingleInstance` to use these new provider methods, allowing tests to properly mock or simulate the cleanup phase of dead socket files.
Adds an optional
exitOverridefunction parameter tounixSingleInstanceto intercept calls toexit(). This allows testing scenarios that normally terminate the process without actually killing the test runner. When provided, the override is called with the intended exit code, and the function returnsfalse.Also adds a test case to verify
exitOverrideis correctly triggered when a second instance tries to start.PR created automatically by Jules for task 15427993918389103174 started by @arran4