Tag: TDD

A NuGet List item parser – Part 2: Including Pre-release packages

In Part 1 I reorganised my NuGet source synchronization tool’s code to make it testable, with the specific aim of allowing pre-release packages to be synchronized. In particular, the affected code was isolated into this function:

function Parse-PackageItem {
	param(
		[string]$Package
	)
	$idVer = $Package.Split(' ')
	if ($idVer.Length -eq 2) {
		$id = $idVer[0]
		[string]$ver = $idVer[1]
		$parts = $ver.Split('.')
		if ($parts.Length -eq 3) {
			[int]$major = $parts[0]
			[int]$minor = $parts[1]
			[int]$patch = $parts[2]
			new-object -TypeName PSCustomObject -Property @{
				package = $Package
				id = $id
				version = $ver
				major = $major
				minor = $minor
				patch = $patch
			}
		}
	}
}

This is a challenge because the pre-release part of the version starts with a hyphen, followed by a series of dot separated identifiers after that. Each identifier consists of letters, digits and hyphens.

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92.

https://semver.org/#spec-item-9

For our purposes, the pre-release label is whatever follows the hyphen.

Creating the first tests

This is the Pester test that we created previously:

if (Get-Module SyncNuGetRepos -All) {
	Remove-Module SyncNuGetRepos
}
Import-Module "$PSScriptRoot\..\bin\Debug\SyncNuGetRepos\SyncNuGetRepos.psm1" -WarningAction SilentlyContinue
Describe "Parse-PackageItem" {
	Context "Exists" {
		It "Runs" {
			Parse-PackageItem
		}
	}
}

First I add a test for the non-pre-release case. This would exist if TDD was done from the beginning.

Context "Non pre-release Item parts" {
		$parts = Parse-PackageItem -Package 'package 1.0.123'
	It "Given 'package 1.0.123', <property&gt; should be <value&gt;" -TestCases @(
		@{ property = 'id'; value = 'package' }
		@{ property = 'major'; value = '1' }
		@{ property = 'minor'; value = '0' }
		@{ property = 'patch'; value = '123' }
		@{ property = 'prerelease'; value = '' }
	) {
		param ($property, $value)
		"$($parts.$property)" | should -Be $value
	}
}

  Context Non pre-release Item parts
    [+] Given 'package 1.0.123', 'id' should be 'package' 69ms
    [+] Given 'package 1.0.123', 'major' should be '1' 23ms
    [+] Given 'package 1.0.123', 'minor' should be '0' 14ms
    [+] Given 'package 1.0.123', 'patch' should be '123' 15ms
    [+] Given 'package 1.0.123', 'prerelease' should be <empty&gt; 14ms

The tests pass.

The good thing is that we can easily modify this test for the other scenarios we want to test. For the next test I am adding a simple pre-release label:

	Context "Simple pre-release Item parts" {
			$parts = Parse-PackageItem -Package 'package 1.0.123-PreRelease'
		It "Given 'package 1.0.123', <property&gt; should be <value&gt;" -TestCases @(
			@{ property = 'id'; value = 'package' }
			@{ property = 'major'; value = '1' }
			@{ property = 'minor'; value = '0' }
			@{ property = 'patch'; value = '123' }
			@{ property = 'prerelease'; value = 'PreRelease' }
		) {
			param ($property, $value)
			"$($parts.$property)" | should -Be $value
		}
	}

As expected, this failed:

  Context Simple pre-release Item parts
    [-] Error occurred in Context block 173ms
      FormatException: Input string was not in a correct format.
      PSInvalidCastException: Cannot convert value "123-PreRelease" to type "System.Int32". Error: "Input string was not in a correct format."
      RuntimeException: Cannot convert value "123-PreRelease" to type "System.Int32". Error: "Input string was not in a correct format."
      at Parse-PackageItem, C:\VSTS\ContinuousIntegration\SyncNuGetRepos\SyncNuGetRepos\bin\Debug\SyncNuGetRepos\SyncNuGetRepos.psm1: line 109
      at <ScriptBlock&gt;, C:\VSTS\ContinuousIntegration\SyncNuGetRepos\SyncNuGetRepos\Tests\Parse-PackageItem.tests.ps1: line 25
      at DescribeImpl, C:\Program Files\WindowsPowerShell\Modules\Pester\4.3.1\Functions\Describe.ps1: line 161

The immediate error was the integer conversion, suggesting that the patch should be fixed. Instead, the pre-release label modifies the entire version, not just the patch. I change

		[string]$ver = $idVer[1]
		$parts = $ver.Split('.')

becomes

		[string]$ver = $idVer[1]
		$verPreRel = $ver.Split('-')
		[string]$verParts = $verPreRel[0]
		[string]$preRel = ''
		if ($verParts.Length -eq 2) {
			$preRel = $verPreRel[1]
		}
		$parts = $verParts.Split('.')

and I assign $preRel to the prerelease property:

function Parse-PackageItem {
	param(
		[string]$Package
	)
	$idVer = $Package.Split(' ')
	if ($idVer.Length -eq 2) {
		$id = $idVer[0]
		[string]$ver = $idVer[1]
		$verPreRel = $ver.Split('-')
		[string]$verParts = $verPreRel[0]
		[string]$preRel = ''
		if ($verPreRel.Length -eq 2) {
			$preRel = $verPreRel[1]
		}
		$parts = $verParts.Split('.')
		if ($parts.Length -eq 3) {
			[int]$major = $parts[0]
			[int]$minor = $parts[1]
			[int]$patch = $parts[2]
			new-object -TypeName PSCustomObject -Property @{
				package = $Package
				id = $id
				version = $ver
				major = $major
				minor = $minor
				patch = $patch
				prerelease = $preRel
			}
		}
	}
}

I’m cheating a little. The test failed, and I had to fix a typo.

The next test verifies that the pre-release part can consist of dot-separtated identifiers:

	Context "Dot-separated identifier pre-release Item parts" {
			$parts = Parse-PackageItem -Package 'package 1.0.123-Pre.Release'
		It "Given 'package 1.0.123', <property&gt; should be <value&gt;" -TestCases @(
			@{ property = 'id'; value = 'package' }
			@{ property = 'major'; value = '1' }
			@{ property = 'minor'; value = '0' }
			@{ property = 'patch'; value = '123' }
			@{ property = 'prerelease'; value = 'Pre.Release' }
		) {
			param ($property, $value)
			"$($parts.$property)" | should -Be $value
		}
	}

The test passes. The next test is for the pre-release part containing a hyphen:

	Context "Hyphened identifier pre-release Item parts" {
			$parts = Parse-PackageItem -Package 'package 1.0.123-Pre-Release'
		It "Given 'package 1.0.123', <property&gt; should be <value&gt;" -TestCases @(
			@{ property = 'id'; value = 'package' }
			@{ property = 'major'; value = '1' }
			@{ property = 'minor'; value = '0' }
			@{ property = 'patch'; value = '123' }
			@{ property = 'prerelease'; value = 'Pre-Release' }
		) {
			param ($property, $value)
			"$($parts.$property)" | should -Be $value
		}
	}

As expected, this failed:

    [-] Given 'package 1.0.123', 'prerelease' should be 'Pre-Release' 13ms
      Expected strings to be the same, but they were different.
      Expected length: 11
      Actual length:   0
      Strings differ at index 0.
      Expected: 'Pre-Release'
      But was:  ''
      -----------^
      60: 			"$($parts.$property)" | should -Be $value
      at <ScriptBlock&gt;, C:\VSTS\ContinuousIntegration\SyncNuGetRepos\SyncNuGetRepos\Tests\Parse-PackageItem.tests.ps1: line 60

The value was empty because of $ver.Split(‘-‘) returning more than two parts. I check the String.Split documentation and see that I can make this $ver.Split(‘-‘, 2). After I make the change, the test passes.

Now my function looks like this:

function Parse-PackageItem {
	param(
		[string]$Package
	)
	$idVer = $Package.Split(' ')
	if ($idVer.Length -eq 2) {
		$id = $idVer[0]
		[string]$ver = $idVer[1]
		$verPreRel = $ver.Split('-', 2)
		[string]$verParts = $verPreRel[0]
		[string]$preRel = ''
		if ($verPreRel.Length -eq 2) {
			$preRel = $verPreRel[1]
		}
		$parts = $verParts.Split('.')
		if ($parts.Length -eq 3) {
			[int]$major = $parts[0]
			[int]$minor = $parts[1]
			[int]$patch = $parts[2]
			new-object -TypeName PSCustomObject -Property @{
				package = $Package
				id = $id
				version = $ver
				major = $major
				minor = $minor
				patch = $patch
				prerelease = $preRel
			}
		}
	}
}

A NuGet List item parser – Part 1: Making legacy code testable

I am building a tool to synchronize two NuGet servers. One is hosted on a development server, and the other is in the cloud.

I need to add support for Pre-Release packages. In the first of two parts, I refactor the code to make it testable.

The existing code

This tool started its life as a script, which I started refactoring to avoid duplicate code. This tool needs to find the packages already in each source. The code is still scripty, Particularly when selecting the packages to be synchronized. There are a lot of legacy packages that should not go to the cloud server.

In the middle I have this code to put the package labels into a more useful structure:

[string]$pkg = $_
$idVer = $pkg.Split(' ')
if ($idVer.Length -eq 2) {
	$id = $idVer[0]
	if (-not $package.ContainsKey($id)) {
		[string]$ver = $idVer[1]
		$parts = $ver.Split('.')
		if ($parts.Length -eq 3) {
			[int]$major = $parts[0]
			[int]$minor = $parts[1]
			[int]$patch = $parts[2]
			new-object -TypeName PSCustomObject -Property @{
				package = $pkg
				id = $id
				version = $ver
				major = $major
				minor = $minor
				patch = $patch
			}
		}
	}
}

What the code sees is something like this:

NuGetShared 0.1.132

It also has to work with something like this:

NuGetProjectPacker 0.1.72-Dependencies

The PreRelease tag, -Dependencies in this example, can contain any combination of letters, digits and hyphens after the initial hyphen.

Preparation

I am going to do this as a professional programmer working on legacy code should. The code must be tested thoroughly, and the Test Driven Development (TDD) methodology demands that I write a failing test for any new functionality, then get the test to work.

In an earlier blog I described how I structure a PowerShell script module project. For testing I create a Tests subfolder. I will be using Pester to run my tests, so the naming convention of the files is important. I use the PowerShell Tools extension to Visual Studio which does some of the work for me. My new function will be called Parse-PackageItem. Parse is not an approved verb, but there does not appear to be an alternative.

Creating the test

Visual Studio creates this test for me, and I add a call to the new function without any parameters:

Describe "Parse-PackageItem" {
	Context "Exists" {
		It "Runs" {
			Parse-PackageItem
		}
	}
}

I run it, and it fails as expected

Describing Parse-PackageItem

  Context Exists
    [-] Runs 1s
      CommandNotFoundException: The term 'Parse-PackageItem' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
      at <ScriptBlock&gt;, C:\VSTS\ContinuousIntegration\SyncNuGetRepos\SyncNuGetRepos\Tests\Parse-PackageItem.tests.ps1: line 4

I start fixing the problem by adding the empty function in the Scripts folder. I could make it visible by dot-sourcing it:

. .\Scripts\Parse-PackageItem.ps1

but that will cause problems later. After I build my project, it is in my script module:

Import-Module "$PSScriptRoot\..\bin\Debug\SyncNuGetRepos\SyncNuGetRepos.psm1"

After I run the test, I get the annoying warning:

[WARNING] The names of some imported commands from the module 'SyncNuGetRepos' include unapproved verbs that might make them less discoverable. To find the commands with unapproved verbs, run the Import-Module command again with the Verbose parameter. For a list of approved verbs, type Get-Verb.

I suppress the warning by adding the Warning Action parameter

Import-Module "$PSScriptRoot\..\bin\Debug\SyncNuGetRepos\SyncNuGetRepos.psm1" -WarningAction SilentlyContinue

I also need to allow the test to rerun, and not re-import the module. That causes problems in Pester if there are multiple instances. The right thing to do is to remove it first. We always want to run our tests against the latest version of the module.

if (Get-Module SyncNuGetRepos -All) {
	Remove-Module SyncNuGetRepos
}
Import-Module "$PSScriptRoot\..\bin\Debug\SyncNuGetRepos\SyncNuGetRepos.psm1" -WarningAction SilentlyContinue
Describe "Parse-PackageItem" {
	Context "Exists" {
		It "Runs" {
			Parse-PackageItem
		}
	}
}

I now turn my attention to the function, and add it:

function Parse-PackageItem {
	
}

I build and run the test which succeeds.

Moving the body of code to the function

The function needs one parameter, -Package, which will replace the $pkg variable in the code

function Parse-PackageItem {
	param(
		[string]$Package
	)
}

I copy the code, paste it into the function, and rename $pkg:

function Parse-PackageItem {
	param(
		[string]$Package
	)
	$idVer = $Package.Split(' ')
	if ($idVer.Length -eq 2) {
		$id = $idVer[0]
		if (-not $package.ContainsKey($id)) {
			[string]$ver = $idVer[1]
			$parts = $ver.Split('.')
			if ($parts.Length -eq 3) {
				[int]$major = $parts[0]
				[int]$minor = $parts[1]
				[int]$patch = $parts[2]
				new-object -TypeName PSCustomObject -Property @{
					package = $Package
					id = $id
					version = $ver
					major = $major
					minor = $minor
					patch = $patch
				}
			}
		}
	}
}

At this point I discover a mistake: I have a hash table $package that I copied into the function. It is used as an optimization, to avoid creating the package object if it already exists. I need to move the test out of the function. The small functional change will be that I occasionally create then ignore the package object, instead of not creating it. I can live with that.

The function is now like this:

function Parse-PackageItem {
	param(
		[string]$Package
	)
	$idVer = $Package.Split(' ')
	if ($idVer.Length -eq 2) {
		$id = $idVer[0]
		[string]$ver = $idVer[1]
		$parts = $ver.Split('.')
		if ($parts.Length -eq 3) {
			[int]$major = $parts[0]
			[int]$minor = $parts[1]
			[int]$patch = $parts[2]
			new-object -TypeName PSCustomObject -Property @{
				package = $Package
				id = $id
				version = $ver
				major = $major
				minor = $minor
				patch = $patch
			}
		}
	}
}

and the code it replaces looks like this:

$pkg = Parse-PackageItem -Package $_
if ($pkg -and -not $package.ContainsKey($pkg.id)) {
	$pkg
}

Note that function is fault tolerant, and does not create the package object if the label does not match the desired pattern.

Summary

Quite a lot of work has gone into this before even starting on the “real” work. It is worth it, because the resulting code is simpler. It is simpler because a large block of tangled code has been simplified by removing a cohesive part of the code that did not belong there. The extracted function is exactly the part of the tool that I want to change.

The testing script will justify its existence when I have added tests for edge cases for what it already does, and for the new functionality.

Having made a start, I will continue, adding tests for the rest of the tool’s functionality.

The script could have been good enough. They often are. It is when things start getting complicated that this approach is justified. The script that I had partially developed was a working prototype of some of what I want to achieve.