Repository navigation
test(windows): make the extensionless task test exercise the file it names - #11924
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Windows task tests preserve inherited environment variables across the suite. They use a distinct ChangesWindows task discovery
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change corrects Windows task-discovery coverage without altering product behavior or runtime configuration, so no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e-win/task.Tests.ps1`:
- Around line 61-64: Update the test setup around the extensionless-file test to
capture the original process-level MISE_WINDOWS_EXECUTABLE_EXTENSIONS value in
BeforeAll, then restore it in AfterAll; reassign the saved value when non-null
and remove Env:\MISE_WINDOWS_EXECUTABLE_EXTENSIONS when it was originally null.
- Around line 53-64: Update both extensionless task discovery tests to capture
the `mise tasks ls` output, assert `$LASTEXITCODE` is 0, and then perform the
existing `noexttask` output assertion. Preserve the current expected
presence/absence behavior in the tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e52f19e-4636-4628-99b3-cc1af2b04be2
📒 Files selected for processing (1)
e2e-win/task.Tests.ps1
282678f to
3c11c21
Compare
Greptile SummaryThis PR corrects the Windows extensionless-task test so it validates task discovery rather than accidentally exercising the colliding
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "test(windows): make the extensionless ta..." | Re-trigger Greptile |
…names `It 'executes a task without extension'` was green but never touched the file it named. Two fixtures collapsed to the same task name (`filetask.bat` and `filetask`), and the `.bat` is what ran, so the extensionless path had no coverage at all. Rename the extensionless fixture to `noexttask` so it stops shadowing, and replace the false assertion with the rule that actually governs it: `windows_executable_extensions` is the discovery boundary for a file with no extension and no shebang. The behaviour the old name claimed -- executing an extensionless file directly -- is not reachable with in-box Windows tooling, so the fix belongs in the test, not in mise. Also save and restore the shell settings the suite sets, and `MISE_TRUSTED_CONFIG_PATHS`: Pester runs every suite in one process, so an `AfterAll` that only removes them leaks into the next suite.
3c11c21 to
fda5d0a
Compare
It 'executes a task without extension'is green but never touches the file it names. Found whilechecking a review comment on #11923 that assumed this test covered a working path.
The collision
BeforeAllwrites two fixtures whose task names collapse to the same thing:Giving them different bodies shows which one runs:
The
.batsupplies the task. Rename the extensionless fixture and the original assertion failsimmediately:
There is a second defect behind the first. The working example in this same file,
It 'executes a task in pwsh', sets all three ofwindows_executable_extensions,windows_default_file_shell_argsanduse_file_shell_for_executable_tasks. The broken test setsone, and its value —
"bat"— is not a shell.Why this is not fixed by "make it run the extensionless file"
The behaviour the name claims is not reachable with in-box Windows tooling. Every combination
measured on
2026.8.4-DEBUG windows-x64:mise tasks lsempty —is_executablewants an extension or a shebangMISE_WINDOWS_EXECUTABLE_EXTENSIONS=""or",exe"os error 193— exec'd directlyMISE_USE_FILE_SHELL_FOR_EXECUTABLE_TASKS=1, shellcmd /cpwsh.exe -File/powershell.exe -Filecscript //nologo //E:vbscriptOnly a POSIX shell can execute an extensionless script, and that is not in-box. mise already has
the mechanism for extensionless files: the shebang — covered by
It 'executes a shebang task with bash'two tests below.So the fix belongs in the test, not in mise.
The change
Test-only, one file.
noexttaskso it stops shadowing, and give it adistinguishable body.
windows_executable_extensionsis the discovery boundary. Deterministic, no external shell,
mise tasks lsonly:The exit-code check and the
filetaskpositive control came out of review, and they are thepoint: a bare
Should -Not -BeLikeis exactly the assertion this PR exists to fix. It would passif
mise tasks lscrashed, and it would pass if the fixture had never been written — the sameclass of green-but-empty test as the one being replaced.
BeforeAll/AfterAll.BeforeEachclears thembetween tests inside this
Describe, but Pester runs every suite in one process, so whateverthe last test left set leaked into the next file. Also from review.
Both discovery assertions verified locally against a real
mise tasks ls:filetaskalone bydefault,
filetaskandnoexttaskwith the setting.Coverage does not drop
The removed assertion had nothing behind it, and its neighbours already cover the paths that work:
.batfile taskIt 'executes a task'It 'executes a task in pwsh'(sets all three settings)It 'executes a shebang task with bash'What is added is the discovery boundary, which nothing covered.
Not done
No bash-based test of "extensionless through a POSIX file shell". It would be the literal reading
of the old name, but it depends on which
bash.exethe runner resolves — on the machine this wasmeasured on that is
C:\WINDOWS\system32\bash.exe, the WSL launcher, not Git Bash. I am notbuilding a test on a dependency I cannot inspect.
I also did not run the Pester suite: Windows tests cannot run in a Linux container, so
windows-e2eis the only place that exercises it.
Conflicts
#11923 adds a context to this same file. Whichever lands second should merge them.
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Tests