Improve ValidateLength error message consistency and refactor validation tests by jorgeasaurus · Pull Request #25806 · PowerShell/PowerShell

jorgeasaurus · 2025-08-01T06:47:24Z

PR Summary

This PR improves the consistency of ValidateLength error messages and significantly enhances the quality of validation attribute tests. The changes improve both user experience (clearer error messages) and code maintainability (more reliable, readable tests).

Key improvements include:

Error Message Consistency

Updated ValidateLengthMaxLengthFailure resource string in Metadata.resx to match the format of ValidateLengthMinLengthFailure
Changed format from "The character length of the {1} argument is too long..." to "The character length ({1}) of the argument is too long..."
Both min and max length errors now use consistent language and formatting with actual length in parentheses

Test Quality Improvements

Replaced generic function names (foo) with descriptive PowerShell-compliant names (Test-ArrayCount, Test-NumericRange, Test-StringLength)
Replaced generic parameter names (bar) with meaningful names ($Items, $Number, $InputString)
Updated error assertions to use -PassThru with captured error variables instead of relying on $error[0]
Added comprehensive ValidateLength test context with specific error message format validation

PR Context

The ValidateLength error messages were inconsistent in format:

MinLength: "The character length (3) of the argument is too short..."
MaxLength (old): "The character length of the 5 argument is too long..." ❌

The updated format makes both messages consistent:

MinLength: "The character length (3) of the argument is too short. Specify an argument with a length that is greater than or equal to "5"..."
MaxLength (new): "The character length (5) of the argument is too long. Specify an argument with a length that is shorter than or equal to "2"..." ✅

Additionally, the test refactoring addresses best practices by using -PassThru to capture exceptions directly rather than reading from the global $error array, making tests more reliable and less prone to interference from other test runs.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge. If this PR is a work in progress, please open this as a Draft Pull Request and mark it as Ready to Review when it is ready to merge.

Breaking changes

None
OR
Experimental feature(s) needed
- Experimental feature name(s):

User-facing changes

Not Applicable
OR
Documentation needed
- Issue filed:

Testing - New and feature

N/A or can only be tested interactively
OR
Make sure you've added a new test if existing tests do not effectively test the code changed

(#25566) ## Problem The ValidateLengthMaxLengthFailure error message was displaying incorrectly, showing the length value where the parameter name should appear: - Actual: "The character length of the 5 argument is too long..." - Expected: "The character length of the "argument" argument is too long..." This occurred because the resource string was updated to expect three parameters (MaxLength, actual length, and parameter name) but the code was only passing two parameters, causing the format string to use the length value in place of the parameter name. ## Root Cause The resource string format was changed to include a parameter name placeholder: - Old format: "The character length of the {1} argument..." - New format: "The character length of the "{2}" argument is too long at {1} characters..." But the corresponding code change to pass the parameter name was not implemented. ## Solution 1. Updated ValidateLengthAttribute.ValidateElement() to pass "argument" as the third parameter when throwing ValidationMetadataException for max length validation failures. 2. Updated the ValidateLengthMaxLengthFailure resource string in Metadata.resx to properly format the error message with all three placeholders. While the current architecture doesn't support passing actual parameter names to validation attributes (which would require significant architectural changes), using "argument" as a generic placeholder prevents the confusing formatting error and provides a clear, consistent error message. ## Changes - Modified src/System.Management.Automation/engine/Attributes.cs to pass "argument" as third parameter in ValidateLength max length validation - Updated src/System.Management.Automation/resources/ Metadata.resx to include proper formatting for parameter name placeholder - Added comprehensive tests in test/powershell/engine /Basic/ValidateAttributes.Tests.ps1 to verify the fix ## Testing Added tests to verify: - Maximum length validation error formatting shows "argument" placeholder - Minimum length validation remains unchanged (different format) - Non-string parameter validation works correctly - Valid string lengths pass validation - Error message contains actual parameter name in outer exception The fix ensures error messages are properly formatted and readable while maintaining backward compatibility. Fixes #25566

src/System.Management.Automation/resources/Metadata.resx

…istency

jorgeasaurus · 2025-10-08T05:38:48Z

Thank you @iSazonov for the review feedback. I've updated the PR to make ValidateLengthMaxLengthFailure consistent with ValidateLengthMinLengthFailure format.

Changes:

1. Error Message Format (Metadata.resx:178)

Changed from:

The character length of the "{2}" argument is too long at {1} characters. Shorten the character length so it is fewer than or equal to {0} characters.

To:

The character length ({1}) of the argument is too long. Specify an argument with a length that is shorter than or equal to "{0}", and then try the command again.

2. Code Update (Attributes.cs:872)

Removed the third parameter to match MinLength's two-parameter format:

// Before
throw new ValidationMetadataException(
    "ValidateLengthMaxLengthFailure",
    null,
    Metadata.ValidateLengthMaxLengthFailure,
    MaxLength, len, "argument");

// After
throw new ValidationMetadataException(
    "ValidateLengthMaxLengthFailure",
    null,
    Metadata.ValidateLengthMaxLengthFailure,
    MaxLength, len);

3. Test Update (ValidateAttributes.Tests.ps1:319-320)

Updated test to verify the new consistent format:

# Check the inner exception message is properly formatted and consistent with MinLength format
$error[0].Exception.InnerException.Message | Should -Match 'The character length \(5\) of the argument is too long\.'
$error[0].Exception.InnerException.Message | Should -Match 'shorter than or equal to "2"'

Result:

Both error messages now follow the same consistent pattern:

MinLength:

The character length (3) of the argument is too short. Specify an argument with a length that is greater than or equal to "5", and then try the command again.

MaxLength:

The character length (6) of the argument is too long. Specify an argument with a length that is shorter than or equal to "2", and then try the command again.

iSazonov · 2025-10-08T09:05:49Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+        BeforeAll {
+            $testCases = @(
+                @{
+                    ScriptBlock             = { function foo { param([ValidateLength(2, 5)] [string] $bar) }; foo "a" }


Please replace foo/bar with more informative names.

Please address the comment.

Please replace foo/bar with more informative names.

iSazonov · 2025-10-08T09:06:22Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+
+            { foo "11111" } | Should -Throw -ErrorId "ParameterArgumentValidationError,foo"
+
+            # Check the inner exception message is properly formatted and consistent with MinLength format


Please remove obvious comment.
Below too.

iSazonov · 2025-10-08T09:13:00Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+            { foo "11111" } | Should -Throw -ErrorId "ParameterArgumentValidationError,foo"
+
+            # Check the inner exception message is properly formatted and consistent with MinLength format
+            $error[0].Exception.InnerException.Message | Should -Match 'The character length \(5\) of the argument is too long\.'


We never use localized messages in tests. Please use more formal check like ".+($ExpectedRealLength).+($ExpectedMaxLength)". Also you can remove \n in the string before check.

…consistency

jorgeasaurus · 2025-10-08T13:11:58Z

Thank you @iSazonov

Changes:

Test Update (ValidateAttributes.Tests.ps1:313-323)

Replaced foo/bar with descriptive names - Changed to Test-ValidateLength and Value
Removed obvious comments - Cleaned up unnecessary comments
Used formal regex pattern - Changed from checking localized message text to using pattern .+$$ExpectedRealLength$.+\"$ExpectedMaxLength`"`

It 'ValidateLength error message should be properly formatted' {
    function Test-ValidateLength { param([ValidateLength(0,2)] [string] $Value) $Value }

    $ExpectedMaxLength = 2
    $ExpectedRealLength = 5

    { Test-ValidateLength "11111" } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLength"

    $error[0].Exception.InnerException.Message | Should -Match ".+\($ExpectedRealLength\).+\`"$ExpectedMaxLength\`""
    $error[0].Exception.Message | Should -Match 'Value'
}

This approach avoids relying on localized message text and provides a more robust test that verifies the error message contains the expected length values in the correct format.

iSazonov · 2025-10-08T17:19:42Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+
+            { Test-ValidateLength "11111" } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLength"
+
+            $error[0].Exception.InnerException.Message | Should -Match ".+\($ExpectedRealLength\).+\`"$ExpectedMaxLength\`""


If we check a message for max we should check a message for min too.

iSazonov · 2025-10-08T17:20:52Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+            function Test-ValidateLength { param([ValidateLength(0,2)] [string] $Value) $Value }
+
+            $ExpectedMaxLength = 2
+            $ExpectedRealLength = 5


If it comes from "11111" it is better to evaluate the value from the string.

…ormatting

jorgeasaurus · 2025-10-09T12:26:58Z

Thank you @iSazonov. I've made the following changes:

Changes:

Added min length test - Now tests both max and min length error messages for consistency
Evaluate length from test strings - Using $TestStringTooLong.Length and $TestStringTooShort.Length instead of hardcoded values

It 'ValidateLength error message should be properly formatted' {
    function Test-ValidateLengthMax { param([ValidateLength(0,2)] [string] $Value) $Value }
    function Test-ValidateLengthMin { param([ValidateLength(5,10)] [string] $Value) $Value }

    $TestStringTooLong = "11111"
    $TestStringTooShort = "123"
    $ExpectedMaxLength = 2
    $ExpectedMinLength = 5

    { Test-ValidateLengthMax $TestStringTooLong } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLengthMax"
    $error[0].Exception.InnerException.Message | Should -Match ".+\($($TestStringTooLong.Length)\).+\`"$ExpectedMaxLength\`""
    $error[0].Exception.Message | Should -Match 'Value'

    { Test-ValidateLengthMin $TestStringTooShort } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLengthMin"
    $error[0].Exception.InnerException.Message | Should -Match ".+\($($TestStringTooShort.Length)\).+\`"$ExpectedMinLength\`""
    $error[0].Exception.Message | Should -Match 'Value'
}

iSazonov · 2025-10-10T04:23:05Z

/azp run PowerShell-Windows-Packaging-CI, PowerShell-CI-linux-packaging

azure-pipelines · 2025-10-10T04:23:14Z

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

jorgeasaurus · 2025-10-10T05:15:39Z

/azp run PowerShell-Windows-Packaging-CI, PowerShell-CI-linux-packaging

@iSazonov this command appears to have failed.

Tagging for awareness.

iSazonov · 2025-10-10T06:57:21Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+        It 'Exception: <FullyQualifiedErrorId>:<InnerErrorId>' -TestCases $testCases {
+            param($ScriptBlock, $FullyQualifiedErrorId, $InnerErrorId)
+
+            $ScriptBlock | Should -Throw -ErrorId $FullyQualifiedErrorId


Using $error[0] is not reliable. We can use:

$err = $ScriptBlock | Should -Throw -ErrorId $FullyQualifiedErrorId -PassThru

iSazonov · 2025-10-10T06:57:45Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+        BeforeAll {
+            $testCases = @(
+                @{
+                    ScriptBlock             = { function foo { param([ValidateLength(2, 5)] [string] $bar) }; foo "a" }


Please address the comment.

Please replace foo/bar with more informative names.

iSazonov · 2025-10-10T06:58:42Z

test/powershell/engine/Basic/ValidateAttributes.Tests.ps1

+
+            { Test-ValidateLengthMax $TestStringTooLong } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLengthMax"
+            $error[0].Exception.InnerException.Message | Should -Match ".+\($($TestStringTooLong.Length)\).+\`"$ExpectedMaxLength\`""
+            $error[0].Exception.Message | Should -Match 'Value'


I wonder why we check this.

…or better clarity

jorgeasaurus · 2025-10-11T16:24:17Z

Thank you @iSazonov. I've made the following changes:

Changes Made

1. Replaced `$error[0]` with `-PassThru`

Updated all test assertions throughout the file to use the -PassThru parameter for more reliable error handling:

Before:

$ScriptBlock | Should -Throw -ErrorId $FullyQualifiedErrorId
if ($InnerErrorId) {
    $error[0].exception.innerexception.errorrecord.FullyQualifiedErrorId | Should -Be $InnerErrorId
}

After:

$err = $ScriptBlock | Should -Throw -ErrorId $FullyQualifiedErrorId -PassThru
if ($InnerErrorId) {
    $err.exception.innerexception.errorrecord.FullyQualifiedErrorId | Should -Be $InnerErrorId
}

This change was applied to all contexts: ValidateCount, ValidateRange (ParameterConstructors, User Defined Range, Predefined Range), and ValidateLength.

2. Replaced `foo`/`bar` with Descriptive Names

Updated all test cases with more informative function and parameter names:

ValidateCount Context:

function foo → function Test-ArrayCount
$bar → $Items

ValidateRange Contexts:

function foo → function Test-NumericRange
$bar → $Number

ValidateLength Context:

function foo → function Test-StringLength
$bar → $InputString

Example:

# Before
ScriptBlock = { function foo { param([ValidateCount(2, 3)] [string[]] $bar) }; foo 1,2,3,4 }

# After
ScriptBlock = { function Test-ArrayCount { param([ValidateCount(2, 3)] [string[]] $Items) }; Test-ArrayCount 1,2,3,4 }

3. Removed Outer Exception Message Checks

Removed the unnecessary outer exception message assertions from the ValidateLength test:

Before:

$err = { Test-ValidateLengthMax $TestStringTooLong } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLengthMax" -PassThru
$err.Exception.InnerException.Message | Should -Match ".+\($($TestStringTooLong.Length)\).+\`"$ExpectedMaxLength\`""
$err.Exception.Message | Should -Match 'Value'

After:

$err = { Test-ValidateLengthMax $TestStringTooLong } | Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLengthMax" -PassThru
$err.Exception.InnerException.Message | Should -Match ".+\($($TestStringTooLong.Length)\).+\`"$ExpectedMaxLength\`""

The outer exception message check was redundant since the ErrorId assertion already validates the correct error type, and the inner exception message check validates the actual validation error content.

Result

✅ Explicit and reliable error capture using -PassThru
✅ Descriptive test function and parameter names
✅ Focused assertions without redundant checks

All changes are in test/powershell/engine/Basic/ValidateAttributes.Tests.ps1.

iSazonov · 2025-10-12T11:38:41Z

/azp run PowerShell-Windows-Packaging-CI, PowerShell-CI-linux-packaging

azure-pipelines · 2025-10-12T11:38:49Z

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

iSazonov · 2025-10-12T11:53:44Z

@jorgeasaurus Please update the PR title and description to reflect the PR changes.

jorgeasaurus · 2025-10-12T14:53:01Z

@iSazonov PR title and description have been updated.

microsoft-github-policy-service · 2025-10-13T06:55:51Z

📣 Hey @@jorgeasaurus, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms

iSazonov · 2025-10-13T06:56:08Z

@jorgeasaurus Thanks for your contribution!

…ion tests (PowerShell#25806)

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Aug 8, 2025

jorgeasaurus added 2 commits August 13, 2025 12:03

Merge branch 'master' into master

5bba78a

Merge branch 'master' into master

00536fb

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 6, 2025

iSazonov reviewed Oct 6, 2025

View reviewed changes

src/System.Management.Automation/resources/Metadata.resx Outdated Show resolved Hide resolved

jorgeasaurus added 2 commits October 7, 2025 22:18

Merge branch 'PowerShell:master' into master

a4488d1

Refactor error message for ValidateLength to improve clarity and cons…

030a946

…istency

jorgeasaurus requested a review from iSazonov October 8, 2025 05:39

iSazonov reviewed Oct 8, 2025

View reviewed changes

Refactor ValidateLength test to improve error message formatting and …

b2927ad

…consistency

jorgeasaurus requested a review from iSazonov October 8, 2025 13:12

iSazonov reviewed Oct 8, 2025

View reviewed changes

Refactor ValidateLength tests to improve error handling and message f…

6a12db2

…ormatting

jorgeasaurus requested a review from iSazonov October 9, 2025 12:27

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Oct 10, 2025

iSazonov reviewed Oct 10, 2025

View reviewed changes

jorgeasaurus added 2 commits October 11, 2025 08:45

Merge branch 'PowerShell:master' into master

8214034

Refactor ValidateAttributes.Tests to use descriptive function names f…

064468f

…or better clarity

jorgeasaurus changed the title ~~Fix ValidateLength error message formatting issue~~ Improve ValidateLength error message consistency and refactor validation tests Oct 12, 2025

iSazonov approved these changes Oct 13, 2025

View reviewed changes

iSazonov self-assigned this Oct 13, 2025

iSazonov added Hacktoberfest Potential candidate to participate in Hacktoberfest Hacktoberfest-Accepted Accepted to participate in Hacktoberfest labels Oct 13, 2025

iSazonov merged commit 26bb188 into PowerShell:master Oct 13, 2025
40 checks passed

SIRMARGIN pushed a commit to SIRMARGIN/PowerShell that referenced this pull request Dec 12, 2025

Improve ValidateLength error message consistency and refactor validat…

3363d47

…ion tests (PowerShell#25806)

kilasuit pushed a commit to kilasuit/PowerShell that referenced this pull request Jan 2, 2026

Improve ValidateLength error message consistency and refactor validat…

cf9ae76

…ion tests (PowerShell#25806)


		{ foo "11111" } \| Should -Throw -ErrorId "ParameterArgumentValidationError,foo"

		# Check the inner exception message is properly formatted and consistent with MinLength format


		{ Test-ValidateLength "11111" } \| Should -Throw -ErrorId "ParameterArgumentValidationError,Test-ValidateLength"

		$error[0].Exception.InnerException.Message \| Should -Match ".+\($ExpectedRealLength\).+\`"$ExpectedMaxLength\`""

Conversation

jorgeasaurus commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Error Message Consistency

Test Quality Improvements

PR Context

PR Checklist

Breaking changes

User-facing changes

Testing - New and feature

Uh oh!

Uh oh!

jorgeasaurus commented Oct 8, 2025

Changes:

Result:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgeasaurus commented Oct 8, 2025

Changes:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgeasaurus commented Oct 9, 2025

Changes:

Uh oh!

iSazonov commented Oct 10, 2025

Uh oh!

azure-pipelines bot commented Oct 10, 2025

Uh oh!

jorgeasaurus commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgeasaurus commented Oct 11, 2025

Changes Made

1. Replaced $error[0] with -PassThru

2. Replaced foo/bar with Descriptive Names

3. Removed Outer Exception Message Checks

Result

Uh oh!

iSazonov commented Oct 12, 2025

Uh oh!

azure-pipelines bot commented Oct 12, 2025

Uh oh!

iSazonov commented Oct 12, 2025

Uh oh!

jorgeasaurus commented Oct 12, 2025

Uh oh!

Uh oh!

microsoft-github-policy-service bot commented Oct 13, 2025 • edited by unfurl-links bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jorgeasaurus commented Aug 1, 2025 •

edited

Loading

1. Replaced `$error[0]` with `-PassThru`

2. Replaced `foo`/`bar` with Descriptive Names

microsoft-github-policy-service bot commented Oct 13, 2025 •

edited by unfurl-links bot

Loading