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>, 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.

Advertisement