Skip to content

style: drop named parameters from single-argument calls in test scaffolding#36

Merged
tablackburn merged 1 commit into
mainfrom
style/scope-test-scaffolding-named-parameters
May 27, 2026
Merged

style: drop named parameters from single-argument calls in test scaffolding#36
tablackburn merged 1 commit into
mainfrom
style/scope-test-scaffolding-named-parameters

Conversation

@tablackburn
Copy link
Copy Markdown
Owner

@tablackburn tablackburn commented May 25, 2026

Summary

  • Apply the scoped named-parameter rule to the canonical test scaffolding: name parameters only on calls passing two or more arguments; single-argument calls stay positional.
  • Reverts over-naming (e.g. Test-Path -Path $path, Get-Module -Name $name) back to positional across tests/Help.tests.ps1, tests/Manifest.tests.ps1, tests/Meta.tests.ps1, tests/MetaFixers.psm1, and the tests/Unit/ templates.

This is the canonical-source fix for the over-naming that propagated into the consumer "align test scaffolding" PRs (tablackburn/PlexAutomationToolkit#55, tablackburn/ReScenePS#18); those will re-sync from this once it lands. Pairs with the rule scoping in tablackburn/ai-agent-instruction-modules#25.

What stays named (intentionally)

  • 2+ value-argument callsJoin-Path -Path … -ChildPath …, Get-Content -Path … -Encoding … -Raw, Get-Command -Name … -CommandType ….
  • Test-Path -LiteralPath (…) — position 0 binds to -Path (which does wildcard expansion), so dropping the name would change behavior.
  • Comment-based-help .EXAMPLE calls in MetaFixers.psm1 — left named for teaching clarity.

Test plan

  • All four real-PowerShell files parse with no syntax errors.
  • PSScriptAnalyzer: no new findings vs. HEAD (pre-existing warnings unchanged).
  • CI Pester suite (ubuntu / windows / macOS) green.

Breaking changes

None — behavior-preserving; positional binding confirmed for every reverted call.

Summary by CodeRabbit

  • Style

    • Updated test scripts to use positional arguments for single-parameter cmdlet calls, improving code consistency across the test suite.
  • Documentation

    • Updated changelog to reflect revised parameter naming conventions in test scaffolding.

Review Change Stack

Copilot AI review requested due to automatic review settings May 25, 2026 16:18
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@tablackburn, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 10 minutes and 54 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e00a9c73-d9ba-466f-804c-4a0fb77a0060

📥 Commits

Reviewing files that changed from the base of the PR and between b0b007b and 20c9af0.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • tests/Help.tests.ps1
  • tests/Manifest.tests.ps1
  • tests/Meta.tests.ps1
  • tests/MetaFixers.psm1
  • tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1
  • tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
📝 Walkthrough

Walkthrough

PowerShell template test scaffolding is updated to enforce a consistent named-parameter style: single-argument function calls now use positional arguments, while multi-argument calls retain named parameters. The changelog documents this rule, test infrastructure files (discovery, manifest validation, helper modules) apply the change, and unit test templates implement it in setup and test cases.

Changes

Named Parameter Style Enforcement

Layer / File(s) Summary
Documentation of style change
CHANGELOG.md
Unreleased Changed entry documents the template test-scaffolding update: named parameters removed from single-argument calls; multi-argument and Test-Path -LiteralPath calls retain named parameters.
Test infrastructure and discovery files
tests/Help.tests.ps1, tests/Manifest.tests.ps1, tests/Meta.tests.ps1, tests/MetaFixers.psm1
Test discovery and metadata validation simplify argument passing: Sort-Object, Split-Path, Import-PowerShellDataFile, Get-Module, Remove-Module, Get-ChildItem, Get-Command, Get-Help, Get-Content, Select-String, and custom helper functions now use positional arguments for single-argument invocations.
Unit test templates
tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1, tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
Template setup phases (module loading, path computation, environment variable initialization) and test case invocations now use positional arguments for single-argument function calls to discovered functions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 With whiskers twitching and tests held tight,
We've trimmed the parameters, made the style right—
One argument stands alone and free,
While pairs dance together in harmony.
A cleaner template, from hop to light!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: dropping named parameters from single-argument calls in test scaffolding, which is the primary objective of this pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch style/scope-test-scaffolding-named-parameters

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the template’s Pester test scaffolding to follow a scoped style rule: use positional binding for single-argument calls, and reserve named parameters for calls with two or more value arguments (or where naming is semantically required, e.g., -LiteralPath).

Changes:

  • Dropped named parameters from single-value-argument calls across test scaffolding and unit test templates (e.g., Get-Module $name, Test-Path $path, Import-PowerShellDataFile $path).
  • Kept named parameters where they materially aid correctness or clarity (e.g., multi-argument calls like Join-Path -Path ... -ChildPath ..., and Test-Path -LiteralPath ...).
  • Documented the scaffolding style change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1 Switches single-argument calls (module import/build plumbing + function invocation) to positional.
tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1 Switches single-argument calls (module import/build plumbing + helper invocation) to positional.
tests/MetaFixers.psm1 Drops -Path / single-arg parameter names in internal helper implementations while preserving behavior.
tests/Meta.tests.ps1 Updates MetaFixers import and helper invocations to positional single-argument calls.
tests/Manifest.tests.ps1 Updates module root/version resolution and imports to positional single-argument calls.
tests/Help.tests.ps1 Updates discovery/import plumbing and helper calls to positional single-argument calls; keeps multi-arg calls named.
CHANGELOG.md Records the test scaffolding style adjustment under Unreleased changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…olding

Apply the scoped named-parameter rule to the template's test scaffolding: name
parameters only on calls that pass two or more arguments; single-argument calls
stay positional. Reverts over-naming such as `Test-Path -Path $path`,
`Get-Module -Name $name`, and `Get-Help -Name $command` to positional across the
Help/Manifest/Meta tests, MetaFixers, and the Unit test templates.

A trailing switch counts as an argument, so `Split-Path -Path $x -Parent`,
`Get-Content -Path $x -Raw`, `Get-ChildItem -Path $x -Recurse`, and
`Sort-Object -Property 'Name' -Unique` keep their names, as do genuinely
multi-value calls (`Join-Path`, `Get-Content -Path -Encoding -Raw`,
`Get-Command -Name -CommandType`) and `Test-Path -LiteralPath` (whose positional
slot binds to -Path). Common parameters such as -ErrorAction and -Verbose do not
count toward the threshold.

Canonical-source fix for the over-naming that propagated into the consumer
test-scaffolding PRs; pairs with the rule scoping in
tablackburn/ai-agent-instruction-modules#25.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@tablackburn tablackburn force-pushed the style/scope-test-scaffolding-named-parameters branch from b0b007b to 20c9af0 Compare May 25, 2026 17:07
@tablackburn tablackburn merged commit 01cca97 into main May 27, 2026
11 checks passed
@tablackburn tablackburn deleted the style/scope-test-scaffolding-named-parameters branch May 27, 2026 02:40
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