Skip to content

test(tools): regression test for Windows creationflags double-pass (#29651) - #29944

Closed
Snehal707 wants to merge 1 commit into
NousResearch:mainfrom
Snehal707:fix/29651-local-popen-creationflags
Closed

test(tools): regression test for Windows creationflags double-pass (#29651)#29944
Snehal707 wants to merge 1 commit into
NousResearch:mainfrom
Snehal707:fix/29651-local-popen-creationflags

Conversation

@Snehal707

Copy link
Copy Markdown

Summary

Add a regression test ensuring LocalEnvironment._run_bash passes creationflags only once on Windows.

Related to #29651

What was happening

On Windows, subprocess.Popen() in LocalEnvironment._run_bash received creationflags twice: once as an explicit keyword (CREATE_NO_WINDOW) and again via **_popen_kwargs (windows_hide_flags()). That raised got multiple values for keyword argument 'creationflags', breaking terminal() and write_file() on the local backend.

What changed

How to verify

  1. pytest tests/tools/test_local_popen_creationflags.py -q (on Windows use --timeout-method=thread if needed)
  2. On Windows with latest main: terminal(command="echo hello") and write_file(path="test.txt", content="test") should succeed.

Platforms tested

  • Windows 11 (native), Python 3.11 venv

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure backend/local Local shell execution P3 Low — cosmetic, nice to have labels May 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #29213 — both are test-only regression guards for the creationflags double-pass bug (already fixed by merged #29059). Different test files/approaches.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for adding a focused regression guard. This automated hermes-sweeper review found that the reported Windows double-pass is already fixed on current main.

  • tools/environments/local.py:1074-1087 supplies creationflags through _popen_kwargs and expands it once into subprocess.Popen(...); there is no explicit second creationflags= argument.
  • Commit 05f02640e1f763afb7afdae4efe6b450016a908f is the direct fix, removing the duplicate explicit keyword from this call.
  • The fix is included in release v2026.5.28.
  • @alt-glitch's note correctly identifies this PR as a test-only guard for the behavior already repaired by merged PR fix(windows): drop duplicate creationflags kwarg in LocalEnvironment run_bash #29059.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/local Local shell execution P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants