Skip to content

test: make process cleanup work on Windows - #165

Merged
juliandescottes merged 4 commits into
mozilla:mainfrom
f3tchcodes:test/windows-process-cleanup
Aug 25, 2026
Merged

test: make process cleanup work on Windows#165
juliandescottes merged 4 commits into
mozilla:mainfrom
f3tchcodes:test/windows-process-cleanup

Conversation

@f3tchcodes

Copy link
Copy Markdown
Contributor

Summary

Make the Vitest cleanup work properly on Windows.

The current cleanup uses Unix-only commands like pgrep, pkill, and kill, so running the tests on Windows prints command errors even though the tests themselves still run.

This change keeps the existing cleanup behavior for Linux and macOS, while adding a Windows-specific cleanup path. On Windows, it finds Firefox test instances using the --marionette command-line argument, terminates those process trees with taskkill, and cleans up leftover geckodriver.exe processes without touching normal Firefox windows.

@juliandescottes juliandescottes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

It looks compatible with the changes from #169 . I hope @freema doesn't mind, I think it will be easier to land your PR first and then trim down 169 to be a bit smaller.

A few comments to address first.

Comment thread tests/setup.ts Outdated
Comment thread tests/setup.ts
Comment thread tests/setup.ts Outdated
@f3tchcodes

f3tchcodes commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

All changes have been made! I also renamed the function findWindowsProcessIds() to findMarionetteFirefoxProcessIds() for better readability.

Co-authored-by: Julian Descottes <jdescottes@mozilla.com>
@f3tchcodes
f3tchcodes force-pushed the test/windows-process-cleanup branch from 7d30325 to 4d491ff Compare August 25, 2026 15:57
@f3tchcodes
f3tchcodes force-pushed the test/windows-process-cleanup branch from 4d491ff to 4b1c8fe Compare August 25, 2026 15:58

@juliandescottes juliandescottes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My bad, the findWindowsProcessIds was also used to find geckodriver processes, only the comment was confusing. We need to restore it and fix the comment

Comment thread tests/setup.ts Outdated
Comment thread tests/setup.ts
@f3tchcodes
f3tchcodes force-pushed the test/windows-process-cleanup branch from 4b1c8fe to b56ecad Compare August 25, 2026 16:16
@f3tchcodes

Copy link
Copy Markdown
Contributor Author

Restored! I completely missed it even after reading through the code haha. Thanks for catching it before the merge!

@juliandescottes juliandescottes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a comment to update and we should be good to go, thanks!

Comment thread tests/setup.ts Outdated
Co-authored-by: Julian Descottes <jdescottes@mozilla.com>
@f3tchcodes
f3tchcodes force-pushed the test/windows-process-cleanup branch from 9ad6c77 to b13525b Compare August 25, 2026 17:47

@juliandescottes juliandescottes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@juliandescottes
juliandescottes merged commit a61502f into mozilla:main Aug 25, 2026
1 check passed
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