Skip to content

test: improve temp dir disposal for CI vs Local#3218

Merged
marvinhagemeister merged 1 commit into
freshframework:mainfrom
csvn:test/improve-temp-dir-handling
Aug 15, 2025
Merged

test: improve temp dir disposal for CI vs Local#3218
marvinhagemeister merged 1 commit into
freshframework:mainfrom
csvn:test/improve-temp-dir-handling

Conversation

@csvn

@csvn csvn commented Aug 15, 2025

Copy link
Copy Markdown
Contributor
  • Cleaning up TMP files in CI is unnecessary, as files are discarded automatically between runs (except of using cache/artifacts)
  • On Windows, <user>\AppData\Local\Temp\* files are not automatically removed (see resources below)

This PR should make tests a tiny bit faster in CI by avoiding work, as well as skipping error messages that might follow.

And running tests locally will now log warnings if temp files fails to be removed instead of silently continuing.

Resources

image

Partially replaces #2990

dir,
async [Symbol.asyncDispose]() {
// Skip pointless cleanup in CI, speed up tests
if (Deno.env.get("CI") === "true") return;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

GitHub Actions (and most other CI tools) sets this to true when run.

https://docs.github.com/en/actions/reference/workflows-and-actions/variables#default-environment-variables

// Ignore errors Files in tmp will be cleaned up by the OS
// Temp files are not cleaned up automatically on Windows
// deno-lint-ignore no-console
console.warn(`Failed to clean up temp dir: "${dir}"`);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can be helpful when running locally, because if failing to remove the dirs it may spam the <user>\AppData\Local\Temp dir with new folders on WIndows that are not cleaned up if running consecutive tests.

@marvinhagemeister marvinhagemeister left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks!

@marvinhagemeister marvinhagemeister merged commit 7107117 into freshframework:main Aug 15, 2025
7 checks passed
@csvn csvn deleted the test/improve-temp-dir-handling branch August 15, 2025 11:31
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