Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow user to specify arguments to be passed to test files #271

Closed
dlwyatt opened this issue Jan 27, 2015 · 9 comments
Closed

Allow user to specify arguments to be passed to test files #271

dlwyatt opened this issue Jan 27, 2015 · 9 comments
Assignees

Comments

@dlwyatt
Copy link
Member

dlwyatt commented Jan 27, 2015

Example usage:

$tests = @(
    @{ Path = "$root\test1.Tests.ps1"; Parameters = @{ Parameter1 = 'Value1'; Parameter2 = 'Value2' } }
    "$root\test2.Tests.ps1"
)
Invoke-Pester -Path $tests
@dlwyatt dlwyatt self-assigned this Jan 27, 2015
dlwyatt added a commit to dlwyatt/Pester that referenced this issue Jan 27, 2015
This is a first crack at resolving issues pester#270 and pester#271 .  I haven't written any tests around this new code yet, but it does successfully run all of Pester's existing tests (except for those looking specifically for the -Path parameter on New-PesterState; that's been removed since nothing but Invoke-Pester was actually looking at it.)
@dlwyatt dlwyatt mentioned this issue Jan 27, 2015
@vors
Copy link
Member

vors commented Jan 27, 2015

I have another usage proposal:

Invoke-Pester -Path .\foo.Tests.ps1 -Parameters = @{ Parameter1 = 'Value1'; Parameter2 = 'Value2' } 

If somebody want to run more then 1 test file, he can call Invoke-Pester twice and merge results.

@vors
Copy link
Member

vors commented Jan 27, 2015

I tried to merge results, and I cannot find an API to do it. Does it exist?

@dlwyatt
Copy link
Member Author

dlwyatt commented Jan 28, 2015

Nope, not at the moment.

@ghostsquad
Copy link
Contributor

I don't think this is a great idea. I also think it's the author's responsibility to encourage proper testing patterns.

http://googletesting.blogspot.com/2014/07/testing-on-toilet-dont-put-logic-in.html

Unlike production code, simplicity is more important than flexibility in tests. Most unit tests verify that a single, known input produces a single, known output. Tests can avoid complexity by stating their inputs and outputs directly rather than computing them. Otherwise it's easy for tests to develop their own bugs.

My suggestion would be to setup Data-Driven/Parameterized tests like what is described in #179

@dlwyatt
Copy link
Member Author

dlwyatt commented Feb 3, 2015

The author knows jack squat about proper testing patterns. ;) I just make the thing work. This particular feature request came from VMware.

@vors
Copy link
Member

vors commented Feb 3, 2015

Request also came from Microsoft, PowerShell team. We want run same test suite against different VMs in parallel (different VMs for different OS versions). Currently we workaround it with passing global variable, which is ugly.

@ghostsquad
Copy link
Contributor

@vors - That's an interesting use case. Though I would argue that a test config file would be better, and there would be no need to try to support the originally requested feature. Regarding your comment on global variables being ugly: yep, they certainly are, if the global scope is cluttered with variables. That's generally bad practice. There's nothing truely wrong with a global variable, as long as they are used properly. A $global:Variable is essentially equivalent to a Static Property in C#. There's quite a few static/global classes used in C# all the time, namely ConfigurationManager comes to mind, as it does exactly what we are talking about here.

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<startup> 
    <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5" />
</startup>
<appSettings>
<add key ="foo" value ="Sample" />
</appSettings>
</configuration>
var myTestConfigValue = ConfigurationManager.AppSettings[foo];

Consider the following:

Run-Tests.ps1
# Run-Tests.ps1
param (
    [string]$TestName = "*",
    [switch]$Debug,
    [switch]$CurrentContext = $false
)
$ErrorActionPreference = "Stop"
if($Debug){
    $DebugPreference = "Continue"
}

$here = (Split-Path -Parent $MyInvocation.MyCommand.Path)

if($currentContext) {
    Import-Module Pester
    Invoke-Pester -TestName $TestName -Path $here
} else {
    $cmd = 'Set-Location ''{0}''; Import-Module Pester; Invoke-Pester -TestName ''{1}'' -EnableExit;' -f $here, $TestName
    powershell.exe -noprofile -command $cmd
}

this is used to run tests in a nested runspace in order to try to avoid state related problems that occur when tests modify global/parent scopes. Though, given nature of Pester, there will always be the chance of state change affecting tests, because there's no isolation between test cases.

Foo.Tests.ps1
# Foo.Tests.ps1
$here = Split-Path -Parent $MyInvocation.MyCommand.Path
. "$here\TestCommon.ps1"

Describe ...

demonstration of including some boilerplate at the top of every test file

TestCommon.ps1
# TestCommon.ps1
$private:here = Split-Path $MyInvocation.MyCommand.Path -Parent

# compute values for configuration here
. $here\test.config.ps1

# alternative get values from static configuration file (which maybe that is generated as a build action?):
$private:jsonPath = Join-Path $here 'JsonFile.json'
$global:TestConfiguration = (Get-Content $jsonPath) -join "`n" | ConvertFrom-Json

this boiler plate code can set some global state before each test file is invoc'd. A great way to get test configuration loaded, and maintained between tests.

@vors
Copy link
Member

vors commented Feb 4, 2015

@ghostsquad Thank you for your thought. Let me provide a little bit more context.

We want our test cases to be both developer friendly and automation friendly.
Developer friendly mean developer should be able to easily provide a VM on which he or she want to run tests. It's important, because part of the development happens on VM. Changin configuration file is not easier. We used to have json configuration files wih our previous test suite. You need to remember to revert changes in this file, before commit it. You need to manipulate only primitive types, like strings and so on.

It also should be easily explorable for new developers. Configuration file is not very explorable, because requiring reading the code and understand the whole test harness.

Consider such code, if we allow pass parameters to tests:

RunInParallel.Tests.ps1
// this file runs everything end to end on all platforms.

#setup $VMs = @($vm1, $vm2, $vm3)
$pesterResults = $VMs | Invoke-Parallel {
          Invoke-Pester -Path Basic.ps1 -Arguments $_ -PassThru
}
# Ideally merge results from all pester runs
# $pesterResults | merge
# for now just throw if some failed
$pesterResults | %{
    if ($_.FailedCount -gt 0) { throw "$($_.FailedCount) tests failed."}
}
# cleanup $VMs

Basic.ps1
// note, it's not Basic.Tests.ps1, because I don't want pester to automatically run this file. That's why #270

<#
    This suite runs basic tests agains specified VM.
    It's the callers responsobility to setup VM, Import Azure module and clean it up after all.
#>

param(
    [Parameter(Mandatory)]
    [Microsoft.WindowsAzure.Commands.ServiceManagement.Model.PersistentVMRoleContext]$vm
)

Describe "behaviour" {
}

Now it's both developer friendly: developer can pass parameter, without changing xml or global state. And automation friendly: simple call Invoke-Pester from root will execute everything in parallel.

@ghostsquad
Copy link
Contributor

#270 is a great feature request. I think I changed my mind. Being able to pass arguments to Invoke-Pester does allow for some interesting automation and command line friendliness.

👍

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

No branches or pull requests

3 participants