fix: address PR #60 review feedback

- SizeParser: wrap TB-suffix overflow in try/catch so callers get
  ArgumentException with the parameter name rather than OverflowException.
- New-PveVm/New-PveContainer: validate -DiskSize/-RootFsSize before
  ShouldProcess so typos like "512M" are rejected even with -WhatIf and
  even when the matching -DiskStorage/-RootFsStorage is omitted.
- Add Pester tests for the new DiskSize and RootFsSize validation paths,
  including a new New-PveContainer.Tests.ps1.
- Add SizeParserTests coverage for the TB overflow path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clint Branham
2026-05-20 18:06:13 -05:00
parent 6181c8ce77
commit fa361a3691
7 changed files with 140 additions and 11 deletions
@@ -76,5 +76,15 @@ namespace PSProxmoxVE.Core.Tests.Utilities
Assert.Equal("DiskSize", ex.ParamName);
Assert.Contains("DiskSize", ex.Message);
}
[Fact]
public void NormalizeToGibibytes_TerabyteOverflow_ThrowsArgumentException()
{
// long.MaxValue with a T suffix overflows when multiplied by 1024.
var input = long.MaxValue.ToString(System.Globalization.CultureInfo.InvariantCulture) + "T";
var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input, "DiskSize"));
Assert.Equal("DiskSize", ex.ParamName);
Assert.Contains("too large", ex.Message, StringComparison.OrdinalIgnoreCase);
}
}
}
@@ -0,0 +1,68 @@
#Requires -Module Pester
<#
.SYNOPSIS
Pester 5 tests for New-PveContainer.
All tests are fully offline — no live Proxmox VE target is required.
#>
BeforeAll {
. $PSScriptRoot/../_TestHelper.ps1
}
Describe 'New-PveContainer' {
Context 'Command existence' {
It 'Should be available after module import' {
Get-Command 'New-PveContainer' -ErrorAction SilentlyContinue |
Should -Not -BeNullOrEmpty
}
It 'Should be a CmdletInfo (binary cmdlet)' {
(Get-Command 'New-PveContainer').CommandType | Should -Be 'Cmdlet'
}
}
Context 'ShouldProcess support' {
BeforeAll {
$script:Cmd = Get-Command 'New-PveContainer'
}
It 'Should support ShouldProcess (WhatIf parameter present)' {
$script:Cmd.Parameters.ContainsKey('WhatIf') | Should -BeTrue
}
It 'Should support ShouldProcess (Confirm parameter present)' {
$script:Cmd.Parameters.ContainsKey('Confirm') | Should -BeTrue
}
}
Context 'RootFsSize validation' {
# Validation runs before ShouldProcess so -WhatIf is enough to exercise it
# without an active session.
It 'Should reject sub-GB units (e.g. 512M)' {
{ New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '512M' -WhatIf -ErrorAction Stop } |
Should -Throw '*unsupported unit*'
}
It 'Should reject sub-GB units even when -RootFsStorage is omitted' {
{ New-PveContainer -Node 'pve-node1' -RootFsSize '512M' -WhatIf -ErrorAction Stop } |
Should -Throw '*unsupported unit*'
}
It 'Should reject malformed input (e.g. 8.5G)' {
{ New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8.5G' -WhatIf -ErrorAction Stop } |
Should -Throw '*not a valid size*'
}
It 'Should accept a bare integer with -WhatIf' {
{ New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8' -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
It 'Should accept "8G" with -WhatIf' {
{ New-PveContainer -Node 'pve-node1' -RootFsStorage 'local-lvm' -RootFsSize '8G' -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
}
}
@@ -133,4 +133,39 @@ Describe 'New-PveVm' {
Should -Throw '*No active Proxmox VE session*'
}
}
Context 'DiskSize validation' {
# Validation runs before ShouldProcess so -WhatIf is enough to exercise it
# without an active session.
It 'Should reject sub-GB units (e.g. 512M)' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '512M' -WhatIf -ErrorAction Stop } |
Should -Throw '*unsupported unit*'
}
It 'Should reject sub-GB units even when -DiskStorage is omitted' {
{ New-PveVm -Node 'pve-node1' -DiskSize '512M' -WhatIf -ErrorAction Stop } |
Should -Throw '*unsupported unit*'
}
It 'Should reject malformed input (e.g. 32.5G)' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32.5G' -WhatIf -ErrorAction Stop } |
Should -Throw '*not a valid size*'
}
It 'Should accept a bare integer with -WhatIf' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
It 'Should accept "32G" with -WhatIf' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32G' -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
It 'Should accept "1T" with -WhatIf' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '1T' -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
}
}