Skip to content

Log temp-dir contents when test teardown fails to delete it#368

Merged
TwitchBronBron merged 3 commits into
masterfrom
task/diagnose-temp-dir-teardown
Jun 23, 2026
Merged

Log temp-dir contents when test teardown fails to delete it#368
TwitchBronBron merged 3 commits into
masterfrom
task/diagnose-temp-dir-teardown

Conversation

@chrisdp

@chrisdp chrisdp commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Diagnostic for the intermittent Windows CI failure where a test's afterEach fails to delete the shared .tmp dir:

Error: ENOTEMPTY: directory not empty, rmdir '...\.tmp'

The victim spec varies (LocationManager, BreakpointManager, ...) and it only happens on Windows, which means an earlier test leaves a file/handle open in the shared .tmp and whichever teardown runs while it's held is the one that throws. We can't tell which leftovers are locking it from the failure alone.

This wraps the .tmp remove/empty calls in the global teardown and the specs that share the dir with a helper that, on failure, logs every path still present under .tmp (and which teardown hit it), then re-throws. It diagnoses the failure without masking it.

  • No production code changes; test-infra only.
  • Silent on success (verified: full suite passes locally with zero diagnostic output).
  • Intended to be reverted once the next Windows failure names the offending leftovers.

The shared .tmp teardown intermittently fails on Windows with ENOTEMPTY when an
earlier test leaves a file/handle open. Wrap the remove/empty calls so that on
failure we log every path still present under .tmp (and which teardown hit it),
then re-throw. Diagnostic only; no production changes.

@TwitchBronBron TwitchBronBron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make this more generic:

  • removeDir or forceDeleteDir or deleteDirWithLogging
  • log a stack trace if it fails rather than just the label (but you can pass an optional label if you really want to)
  • also could provide the number of times it should retry deleting the dir before giving up. (you can leave this as 1 for now if you're really interested in knowing what's blocking it, but ultimately i just want the tests to pass and force the dir to be empty)

TwitchBronBron and others added 2 commits June 22, 2026 11:03
Replace the removeTempDir/emptyTempDir helpers with a single generic
forceDeleteDir(dir, options?) that retries the delete (retryCount,
default 1), takes an optional label, and on failure logs the remaining
paths plus the error message and stack trace before re-throwing.
@chrisdp chrisdp requested a review from TwitchBronBron June 23, 2026 13:35
@TwitchBronBron TwitchBronBron merged commit c3af172 into master Jun 23, 2026
10 checks passed
@TwitchBronBron TwitchBronBron deleted the task/diagnose-temp-dir-teardown branch June 23, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants