listIosDevices can leave its temp directory behind, and the test that guards it asserts on the shared tmpdir
- Lingua principale
- TypeScript
- Stelle
- 41
- Fork
- 1
- Merge medio
- 1h 25m
- PR unite (30g)
- 397
Descrizione
Two problems, found because the second one failed the unit suite on a developer machine while CI stayed green.
## 1. The temp directory leaks from real runs
`listIosDevices` writes `devicectl` output into a `stim-devicectl-*` directory under `tmpdir()` and removes it afterwards. After an iOS `caches` e2e run, one survived with its payload intact:
```
drwx------ 3 janicduplessis staff 96 Sep 11 08:17 /var/folders/.../T/stim-devicectl-YRNy7x
-rw-r--r-- 1 janicduplessis staff 487 Sep 11 08:17 devices.json
```
So the removal is skipped on some path — most likely when the process is interrupted or the call throws between writing and cleaning up. Each leak is small, but they accumulate silently in `tmpdir()` and are invisible to `stim gc`, which does not survey them.
## 2. The guard asserts on global state, so anything else on the machine can fail it
`packages/stim-cli/src/__tests__/engine-ios-device.test.ts:281`:
```ts
expect(readdirSync(tmpdir()).some((e) => e.startsWith('stim-devicectl-') && existsSync(join(tmpdir(), e)))).toBe(
false,
);
```
and at `:692` the same idea as a deep-equality against `[]`. These read the **shared** system temp directory, not a directory the test owns. Any leaked directory from any earlier or concurrent Stim process fails them, regardless of whether the code under test behaved correctly.
Observed as:
```
FAIL engine-ios-device.test.ts > listIosDevices runs devicectl into a temp file, parses it, and removes the directory
AssertionError: expected true to be false
FAIL engine-ios-device.test.ts > listIosDevices against a real devicectl > the argv is accepted and every entry it returns is well formed
AssertionError: expected [ 'stim-devicectl-YRNy7x' ] to deeply equal []
```
Removing the single leaked directory made the file pass with nothing else changed — 131/131 test files green afterwards.
This is why CI never sees it: a fresh runner has an empty `tmpdir()`. It is reproducible for anyone who has run Stim on their machine, and it presents as a failure in code the developer did not touch, which is the expensive kind of false signal.
## Suggested fix
For 2, have the test scope its assertion to directories it created — record the paths the stubbed executor was handed and assert those are gone — rather than scanning a directory it does not own.
For 1, remove the directory in a `finally` so an interrupted or throwing call cannot leave it, and consider having `gc` report `stim-devicectl-*` residue so existing leaks are reclaimable.
Both predate 1.1.0.
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Valutazione
Questa issue non è ancora stata valutata.