Skip to content

chore(install-k8s): drop dead Get-WslConfigContent + its test - #517

Merged
shujaatTracebloc merged 2 commits into
developfrom
chore/drop-dead-get-wslconfigcontent
Jul 31, 2026
Merged

chore(install-k8s): drop dead Get-WslConfigContent + its test#517
shujaatTracebloc merged 2 commits into
developfrom
chore/drop-dead-get-wslconfigcontent

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes Get-WslConfigContent from scripts/install-k8s.ps1 and its Pester test. The function had no production caller — it was superseded by Add-WslMemorySetting and stayed alive only because its own test referenced it.

Spotted while fixing the .wslconfig memory floor in #516.

Why it's dead

Get-WslConfigContent built a whole .wslconfig body from scratch, returning a fixed [wsl2] + memory=<N>GB stanza. That clobbers any other settings the operator had (processors, swap, …).

Add-WslMemorySetting replaced it: it merges a memory= line into existing content, preserves other sections, and returns $null when a memory= is already present so operator tuning is never overwritten.

Set-DailyUserProvisioning — the only consumer in the #418 daily-user-provisioning path — calls Get-WslConfigMemoryGb + Add-WslMemorySetting. Nothing calls the old builder.

The one remaining reference was its Pester test, which asserted that the function works, not that anything uses it — so the test was the only thing keeping the dead code alive. Both removed together.

Caller re-verification

Re-grepped before removing, not just scripts/:

SweepHits
grep -rn "Get-WslConfigContent" scripts/definition + 1 test only
grep -rn "Get-WslConfigContent" .github/none
whole repo, excl. .gitdefinition + 1 test only
case-insensitive wslconfigcontent, whole reposame 3 lines
grep -rn "WslConfig" (to catch dynamic invocation via & / Invoke-Expression)no indirect invocation

No workflow shells into it. Safe to delete.

Rebased onto #516

#516 landed on develop while this was open and rewrote Get-WslConfigMemoryGb — the function immediately above this deletion — so the hunks overlapped and this PR went conflicting. Rebased onto bb4feab.

Both .ps1 files auto-merged; the only real conflict was the install-k8s.ps1 line in scripts/manifest.sha256, where both commits changed the same hash. Resolved by re-running gen-manifest.sh rather than picking a side — the merged file matches neither parent's hash, so either side would have been wrong. Verified the committed hash equals shasum -a 256 scripts/install-k8s.ps1.

Re-verified the premise on post-#516develop before continuing: Get-WslConfigContent still had no caller there, and #516 touched its test only as diff context (no added dependency). #516's rewritten Get-WslConfigMemoryGb and all 8 of its new assertions are intact on this branch.

Test plan

All repo gates re-run against the rebased tree, green:

  • Invoke-Pester scripts/tests/376 passed, 0 failed, 9 skipped (incl. all installer scripts verified against the signed manifest). fix(#418): .wslconfig memory can't be written below the client's own floor #516 reported 377; this removes exactly one test.
  • Invoke-ScriptAnalyzer -Path scripts/install.ps1, scripts/install-k8s.ps1 -Severity Error,Warning0 Errors (236 Warnings, all pre-existing style rules). Compared against the new base: BASE (bb4feab): Errors=0 Warnings=236 / HEAD (rebased): Errors=0 Warnings=236 — this PR introduces no new findings.
  • bash scripts/check-style.shok: style + terminology clean
  • bash scripts/tests/check-drift.shno drift.
  • bash scripts/gen-manifest.sh — regenerated, scripts/manifest.sha256 committed (R8 gate)

Diff is 3 files: −6 lines in install-k8s.ps1, −5 in the test, 1 hash line in the manifest.

Local-environment notes, both unrelated to this change. (1) The two pwsh gates had to be run against a copy of scripts/ at a space-free path — PowerShell 7.5.2's filesystem provider silently enumerates zero entries under this checkout's parent dir (Claude File System), so the literal gate commands report "No test files were found" there rather than failing loudly. CI checkout paths have no spaces. (2) Invoke-ScriptAnalyzer -Path in PSScriptAnalyzer 1.25.0 rejects a two-path array (Cannot convert 'System.Object[]' to ... 'System.String') and returns empty, which reads as a false pass — the numbers above come from running it once per file. Worth checking that CI doesn't invoke it in the two-path form.

Type

Tech-debt — dead-code removal. No behavior change.

🤖 Generated with Claude Code


Note

Low Risk
Dead-code removal only; production WSL paths are unchanged and tests still cover Add-WslMemorySetting.

Overview
Removes dead helper Get-WslConfigContent from scripts/install-k8s.ps1 and drops its Pester coverage. That helper built a full .wslconfig from scratch; daily-user WSL memory sizing already goes through Get-WslConfigMemoryGb + Add-WslMemorySetting in Set-DailyUserProvisioning, which merges memory= without overwriting other settings.

Updates scripts/manifest.sha256 for the changed install-k8s.ps1. No installer behavior change.

Reviewed by Cursor Bugbot for commit 4b81c48. Bugbot is set up for automated code reviews on this repo. Configure here.

Get-WslConfigContent had no production caller. It was superseded by
Add-WslMemorySetting, which merges a memory= line into EXISTING
.wslconfig content instead of overwriting it -- so it preserves other
tuning (processors, swap, ...). Set-DailyUserProvisioning calls
Get-WslConfigMemoryGb + Add-WslMemorySetting; nothing calls the old
whole-file builder.
The only remaining reference was its own Pester test, which asserted the
function worked rather than that anything used it -- so the test kept
dead code alive. Removed both.
Re-verified no caller across scripts/ and .github/ (incl. a
case-insensitive sweep and a check for dynamic invocation) before
removing.
Regenerated scripts/manifest.sha256 for the R8 static-analysis gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka
LukasWodkaforce-pushed the chore/drop-dead-get-wslconfigcontent branch from d925e5e to 4c23461CompareJuly 31, 2026 13:13

@shujaatTraceblocshujaatTracebloc 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.

Approving — clean dead-code removal, independently verified.

What I checked

  • Dead-code premise holds.grep -rniI Get-WslConfigContent (and case-insensitive wslconfigcontent) over the whole tree returns zero matches after the removal — the definition and its lone Pester test were the only references.
  • Live path is unaffected.Set-DailyUserProvisioning (install-k8s.ps1:1945–1966) provisions .wslconfig via Get-WslConfigMemoryGb + Add-WslMemorySetting (the merge-preserving replacement), never the removed builder. No behavior change.
  • R8 manifest gate. Committed hash for scripts/install-k8s.ps1 equals shasum -a 256 of the file. ✓
  • Diff is exactly as described — −6 in install-k8s.ps1, −5 in the test, 1 hash line in the manifest.
  • CI fully green (Pester, ScriptAnalyzer/Static analysis, drift, all Prereqs/PATH/E2E matrices) and Cursor Bugbot reports no findings.

Brought the branch current with develop (merged the docs-only RFC-0003 commit; manifest still matches, CI re-ran green).

Nice catch pulling this out of #516. LGTM. 🚀

@shujaatTracebloc
shujaatTracebloc merged commit b33f719 into developJul 31, 2026
38 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the chore/drop-dead-get-wslconfigcontent branch July 31, 2026 13:26
LukasWodka added a commit that referenced this pull request Jul 31, 2026
develop moved again while this branch sat waiting for review: #517 (drop the dead
Get-WslConfigContent + its test) and #434 (RFC-CLIENT-0003 docs). Both touch the
same install-k8s.ps1 hash line in scripts/manifest.sha256, which is the only
textual conflict; the PowerShell and test files auto-merged.
Verified the merge rather than trusting it:
- #517's removal came through cleanly -- Get-WslConfigContent is gone from both
the installer and the test file (0 references in each).
- This branch's + #516's accessor work is intact (15 accessor references).
- Pester 391 passed / 0 failed. One fewer than the 392 before this merge, which
is exactly the Get-WslConfigContent test #517 deleted -- not a lost assertion.
Gates: Invoke-ScriptAnalyzer as CI scopes it -> 0 errors; check-style.sh clean;
check-drift.sh no drift; manifest.sha256 regenerated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 31, 2026
develop moved a long way while this sat open (#513 macOS memory floor, #518
storage remedy, #516/#444/#517/#520 on the Windows side, #434 RFC docs). Only
scripts/manifest.sha256 conflicted textually — the same single install-k8s.ps1
hash line that has now collided four times today. preflight.sh and
preflight.bats auto-merged.
Resolution: regenerate the manifest (it is a DERIVED artifact — 18 digests, no
secrets; authenticity comes from the release workflow's cosign signature, not
from git), so regenerating is the only correct resolution. Taking either side
would leave a wrong digest, which the R8 gate then rejects.
Verified the auto-merge rather than trusting it. The real hazard here was not the
conflict but the clean-looking merge: this branch DELETES _pf_total_mem_kb (the
"prefer the runtime" memory selector whose conflation of host RAM and VM budget
is the bug it fixes), so any caller that landed on develop meanwhile would have
merged into a call to a function that no longer exists — a silent break git
reports as success.
- _pf_total_mem_kb: undefined and unreferenced after the merge; the only mentions
are this branch's own guard test asserting its absence, and a comment.
- #518's _pf_storage_type network-FS remedy survived intact.
- This branch's _pf_runtime_mem_status is present and still wired into both
_pf_memory and _pf_recheck_runtime_mem.
Gates: bats scripts/tests/*.bats -> 683 ok / 0 not ok (full TAP plan reported,
not a truncated read); shellcheck --severity=error over the CI file set -> rc=0;
bash -n clean; Pester -> 403 passed / 0 failed (install-k8s.ps1 arrived via this
merge); check-style clean; check-drift no drift; gen-manifest.sh --check current.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

Sign up for freeto 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.

3 participants

@LukasWodka@shujaatTracebloc@saadqbal