Skip to content

test(windows): hide test helper console windows (#3706) - #3710

Closed
rodboev wants to merge 2 commits into
nesquena:masterfrom
rodboev:pr/windows-test-create-no-window
Closed

rodboev wants to merge 2 commits into
nesquena:masterfrom
rodboev:pr/windows-test-create-no-window

Conversation

@rodboev

@rodboev rodboev commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Windows local suite runs currently start several long-lived helper processes through subprocess.Popen, and each unflagged child opens a visible console window.
  • The root cause is repeated launch-site duplication rather than a single fixture bug, so fixing only conftest.py and browser_smoke.py would leave other focus-stealing helpers behind.
  • The narrow upstream-safe fix is to add Windows-only CREATE_NO_WINDOW kwargs at each known helper spawn and leave all non-Windows behavior untouched.

What Changed

  • tests/conftest.py: hide the main test server fixture subprocess on Windows.
  • tests/browser_smoke.py: hide the browser smoke server subprocess on Windows.
  • tests/test_tls_support.py: hide the TLS helper server subprocess on Windows.
  • tests/test_ctl_script.py: hide both long-lived helper subprocesses used by the ctl tests on Windows.

Why It Matters

Windows full-suite runs stop spawning foreground console windows for each helper process, so parallel local testing no longer steals focus or makes the machine unusable. The change is test-only and preserves existing behavior everywhere else.

Verification

pytest tests/test_tls_support.py tests/test_ctl_script.py -v --timeout=60
  • Manual: On Windows, run the targeted tests and confirm none of the helper launches opens a new console window.
  • Windows local baseline note: tests/test_tls_support.py::TestTLSEndToEnd::test_tls_startup_failure_fallback_to_http still fails on clean origin/master because it imports fcntl, which is not available on Windows. This branch does not change that assertion path.

Risks / Follow-ups

  • This only covers the currently known subprocess.Popen test helpers; future long-lived test subprocesses should follow the same Windows launch pattern.

Model Used

GPT-5.5 via Codex CLI

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR suppresses console window popups on Windows for all long-lived subprocess.Popen test helpers by conditionally passing creationflags: subprocess.CREATE_NO_WINDOW at each spawn site.

  • tests/conftest.py, tests/browser_smoke.py, tests/test_tls_support.py: Each gets a single CREATE_NO_WINDOW kwargs injection on its long-lived server subprocess; non-Windows behavior is unchanged.
  • tests/test_ctl_script.py: Same treatment applied to both the fake-launchd bash helper and the Python sleeper used by the stale-PID test.
  • Short-lived subprocess.run calls (e.g., the openssl cert-gen helper) are intentionally out of scope; the PR description explicitly acknowledges this as follow-up territory.

Confidence Score: 5/5

Safe to merge — all changes are test-only, additive, and touch no production code paths.

The patch is narrowly scoped: it adds a Windows-only creationflags kwarg at each known long-lived test subprocess spawn site using the named constant subprocess.CREATE_NO_WINDOW. Non-Windows code paths are completely unaffected by the conditional dict unpacking. No logic, fixtures, or assertions are modified. The previous review comments about the magic number 0x08000000 have been addressed by using the named constant throughout.

No files require special attention.

Important Files Changed

Filename Overview
tests/browser_smoke.py Adds CREATE_NO_WINDOW creationflag via the named constant on Windows to suppress the console window for the long-lived server subprocess.
tests/conftest.py Applies the same CREATE_NO_WINDOW pattern to the test-server fixture subprocess; sys is already imported at the top of this file.
tests/test_ctl_script.py Applies CREATE_NO_WINDOW to both the fake-launchd bash helper and the sleeper Python subprocess; no logic changes.
tests/test_tls_support.py Adds import sys, reorders imports to strict alphabetical order, and applies CREATE_NO_WINDOW to the long-lived TLS server subprocess. Short-lived openssl subprocess.run calls are intentionally left unchanged.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test process starts subprocess.Popen] --> B{sys.platform == 'win32'?}
    B -- Yes --> C[Pass creationflags: CREATE_NO_WINDOW]
    B -- No --> D[Pass no extra flags]
    C --> E[Child process starts without console window]
    D --> F[Child process starts normally]
    E --> G[Tests run / subprocess stays alive]
    F --> G
Loading

Reviews (2): Last reviewed commit: "test(windows): use CREATE_NO_WINDOW cons..." | Re-trigger Greptile

Comment thread tests/browser_smoke.py Outdated
Comment thread tests/conftest.py Outdated
Comment thread tests/test_tls_support.py
@rodboev

rodboev commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@nesquena-hermes Can you fast-track this please? It is killing my workflow.

nesquena-hermes added a commit that referenced this pull request Jun 6, 2026
…indows #3710) (#3753)

* test(windows): hide test helper console windows (#3706)

* test(windows): use CREATE_NO_WINDOW constant (#3706)

* test(windows): hide test-helper console windows on Windows (#3710 fixes #3706)

@rodboev. Long-lived test helper subprocesses (test server, browser-smoke, TLS helper,
ctl helpers) now spawn with CREATE_NO_WINDOW on Windows so a local pytest run doesn't
pop focus-stealing console windows. sys.platform=='win32' guarded → no-op on macOS/Linux
(expands to **{}). Test-only, no production code, no user impact. + CHANGELOG v0.51.301.

---------

Co-authored-by: Rod Boev <rod.boev@gmail.com>
Co-authored-by: nesquena-hermes <[email protected]>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped — thanks @rodboev! 🎉

This landed in v0.51.301 (commit 1649a22f; cherry-picked onto the release stage, so the PR branch itself didn't merge and GitHub left this open). Verified live on origin/master: the long-lived test helper subprocesses (main test server in tests/conftest.py, browser-smoke server, TLS helper, and the ctl test helpers) now spawn with creationflags=subprocess.CREATE_NO_WINDOW under a sys.platform == "win32" guard, so a local pytest run no longer pops up focus-stealing console windows on Windows. No behavior change on macOS/Linux.

Issue #3706 is closed. Closing this PR as shipped-via-cherry-pick. Thanks for fixing all five call sites — the parallel-worktree run is much more usable now.

SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…indows nesquena#3710) (nesquena#3753)

* test(windows): hide test helper console windows (nesquena#3706)

* test(windows): use CREATE_NO_WINDOW constant (nesquena#3706)

* test(windows): hide test-helper console windows on Windows (nesquena#3710 fixes nesquena#3706)

@rodboev. Long-lived test helper subprocesses (test server, browser-smoke, TLS helper,
ctl helpers) now spawn with CREATE_NO_WINDOW on Windows so a local pytest run doesn't
pop focus-stealing console windows. sys.platform=='win32' guarded → no-op on macOS/Linux
(expands to **{}). Test-only, no production code, no user impact. + CHANGELOG v0.51.301.

---------

Co-authored-by: Rod Boev <rod.boev@gmail.com>
Co-authored-by: nesquena-hermes <[email protected]>
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