pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.11k stars 473 forks source link

Pester Gherkin processor assumes too much when locating Step files #1129

Closed fourpastmidnight closed 3 years ago

fourpastmidnight commented 6 years ago

1. General summary of the issue

When the Gherkin feature of Pester attempts to find step definitions for the scenarios in feature files, it makes a bad assumption about the possible location(s) of *.step.ps1 files containing step definitions.

2. Describe Your Environment

Pester version : 4.4.2 C:\Program Files\WindowsPowerShell\Modules\Pester\4.4.2\Pester.psd1 PowerShell version : 5.1.17134.165 OS version : Microsoft Windows NT 10.0.17134.0

3. Expected Behavior

In The Cucumber Book, on page 79 it talks about how one might go about organizing feature files:

Subfolders

This is the easiest way to organize your features. You may find yourself torn as to how to choose a catgeory, though....\ Of course, this is a decision for you and your team to take, but we can offer a little bit of advice. We've had the most success using subfolders to represent different high-level tasks that a user might try to do....

features/
    reading_reports/
    report_building/
    user_administration/

My module will be made up of many "regions" of functionality--and so my module will contain various folders under the Public folder of the module to organize these regions of functionality. So, in the same way, I want to organize my Feature files.

With Cucumber, all step definition files are expected to be located under the features folder. The Cucumber Book recommends a folder named step_definitions. I simply use steps, as do many others. In any event, on page 130, they go on to suggest a method of organizing step definitions:

Organizing Our Step Definitions

We've managed to get this far with a single file of step-definitions called steps.rb, and for a project of this size, we might well stick with a single file for a while longer. As the number of step definitions starts to grow, we'll want to split up the files so that the code is more cohesive. We've found that our favorite way to organize step definition files is to organize them with one file per domain entity. So, in our example, we'd have three files:

features/step_definitions/account_steps.rb
features/step_definitions/teller_steps.rb
features/step_definitions/cash_slot_steps.rb

The reason why there's no further folder organization under the step_definitions directory is because in Cucumber, all step definitions are global to the test suite. (See the sidebar on page 149 in The Cucumber Book.)

So, following along with the previous section, and this section, the file system would be organized as follows:

Specs/
    features/
        reading_reports/
            *.feature
        report_building/
            *.feature
        step_definitions/
            account_steps.rb
            teller_steps.rb
            cash_slot_steps.rb
        user_administration/
            *.feature

I would expect to be able to have a similar folder structure for my own specs and have Pester be able to find all the step definitions. Given that, I too, don't follow the exact convention as set out in The Cucumber Book (I use steps instead of step_definitions),

4.Current Behavior

Currently, Pester is very opinionated about where it expects to find step definitions for a given feature. Here's the relevant code from Gherkin.ps1 on lines 467-468:

# Line 467-468 in Gherkin.ps1
$Parent = & $SafeCommands["Split-Path"] $FeatureFile.FullName
Import-GherkinSteps -StepPath $Parent -Pester $pester

So, given this code, and the folder structure suggested by The Cucumber Book, the step definitions for the, say, reading_reports/ features would be expected to be under the reading_reports folder as well.

# If using the folder structure as suggested by The Cucumber Book, then $Parent becomes:
$Parent = "Path_To_Specs\features\reading_reports"
Import-GherkinSteps -StepPath $Parent -Paster $pester

Since the step definitions are located under features\step{s,_definitions}, the step definitions will never be found.

5. Possible Solution

Gherkin.ps1 needs to be updated to allow for more granular options for specifying where feature files and/or step definition files are located.

It would be a good assumption that if Invoke-Gherkin were called from the root of the module source directory, that the feature files could be found in one of these two places:

Furthermore, given no additional information, it would be a good assumption that Pester could find step definitions for the features in the following locations:

However, not everyone likes to structure their source code in the same way. Currently, Pester allows you to specify a -Path parameter. The -Path parameter, however, is a simple string, not a string[]. This allows you to only specify a single path for finding all files required to run the tests and also requires that Gherkin.ps1 bake in some opinionated assumptions about where files are located.

I would suggest that the -Path parameter be deprecated, and a new -FeaturePath and -StepDefinitionPath be added to Gherkin.ps1 to allow for defining these paths. (For the deprecation of -Path, we could add an alias to the new -FeaturePath parameter so that -Path would be the same as specifying -FeaturePath.) Both of these parameters would be optional, using the defaults specified above. If -FeaturePath is provided, then the default for -StepDefinitionPath would be $FeaturePath/**/*.steps.ps1 (this is just pseudo-code).

6. Context

Due to Pester's opinionated assumption about the location of various Gherkin test files, I'm unable to structure the test files for my module in a way that seems best to me and in a way that conforms to how many in the Cucumber community would organize their spec suite. This will cause friction with a moderately-sized spec suite with the potential number of step definition files that would be created. In fact, even the defaults mentioned above would not quite be the organization I would ultimately choose. I would, in fact, create sub-folders under the Specs/features/steps directory to mimic the folder structure of the features since I tend to have one step definition file per module script for those step definitions that are specific to the module script under test. I suspect this would be unique to this particular module that I'm creating—a module containing all scripts required to deploy our application, which will be installed/imported by our Octopus Deploy targets and then used by the Octopus Deploy deployment process. So having separate folders for each set of scripts responsible for various areas of the deployment process makes sense to me at this time. I also have a Specs/features/steps/Shared.steps.ps1 for, obviously, shared step definitions. But where this is all falling apart for me is:

MyModule/
    MyModule/
        Private/
        Public/
            Utility/
                My-FirstScript.ps1
        MyModule.psd1
        MyModule.psm1
    Specs/
        features/
            hooks/
               hooks.steps.ps1                # <-- This is found ok for the MyModule.feature--but not for My-FirstScript.feature
            steps/
                Utility/
                    My-FirstScript.steps.ps1  # <-- This is not found!!
                MyModule.steps.ps1
                Shared.steps.ps1
            Utility/
                My-FirstScript.feature        #<-- This is found ok
            MyModule.feature
fourpastmidnight commented 6 years ago

Hmm, a simpler option would be to use an "un-adulterated" $Path. In Gherkin.ps1, on line 284 is he following:

foreach ($FeatureFile in & $SafeCommands["Get-ChildItem"] $Path -Filter "*.feature" -Recurse ) {
    Invoke-GherkinFeature $FeatureFile -Pester $pester
}

So, if I give -Path the value C:\src\git\path-to-my-repo\MyModule\Specs\features, then all feature files found under the features folder will be found, regardless of how deeply nested, the folder hierarchy is. Now, if all step definitions are also located under the features folder, then the same value for -Path could be used, albeit with a different filter: *.steps.ps1.

But, I still think it would be better to provide the user the ability to specify one or more paths for each of the Feature files and Step Definition files independently.

fourpastmidnight commented 6 years ago

I just confirmed that the following changes do work, but it feels "hack-ish" to me—I still would like to be able to independently specify the paths for Feature files vs. Step Definition files:

# Gherkin.ps1
function Invoke-Gherkin {
    # ...

    # L: 284-286
    foreach ($FeatureFile in & $SafeCommands["Get-ChildItem"] $Path -Filter "*.feature" -Recurse) {
        Invoke-GherkinFeature $FeatureFile -StepPath $Path -Pester $pester   #<-- Add new -StepPath argument
    }

    # ...
}

function Invoke-GherkinFeature {
    [CmdletBinding()]
    param(
        [Alias("PSPath")]
        [Parameter(Mandatory = $True, Position = 0, ValueFromPipelineByPropertyName = $True)]
        [IO.FileInfo]$FeatureFile,

        [string]$StepPath,    #<-- Add new StepPath parameter

        [PSObject]$Pester
    )

    # ...

    # Line 467-47
    try {
        Import-GherkinSteps -StepPath $StepPath -Pester $Pester    #<-- Use the new parameter here
        $Feature, $Background, $Scenarios = Import-GherkinFeature -Path $FeatureFile.FullName -Pester $Pester
    } catch [Gherkin.ParserException] {
        & $SafeCommands["Write-Error"] -Exception $_.Exception -Message "Skipped '$($FeatureFile.FullName)' because of parser error.`n$(($_.Exception.Errors | & $SafeCommands["Select-Object"] -Expand Message) -join "`n`n")"
        continue
    }

    # ...
}

(FYI: this is more of a "note to self". I've opened up enough issues here that I've now forked this. If I can find a few spare minutes, it'd be a good time to start contributing back.)

fourpastmidnight commented 6 years ago

So my proposal directly above only works if your feature files are not in nested folders--as mine are. So my hunch was correct that a more full-featured solution is adding the ability to specify paths for both the feature files and the step files. (Again, more of another note to self.)

nohwnd commented 6 years ago

@Jaykul could you have a look pls ?:)

Jaykul commented 6 years ago

Currently it's a recursive search of the folder where the .feature file is found. However, the search for .feature files is also recursive ... It's also worth nothing that you can manually call global steps files from feature-level step files (i.e. by adding a . ..\..\global.steps.ps1)

The end result is that you can organize your feature files in folders, but your steps are not automatically shared across those folders.

In order to better support global step files, we could easily add a StepPath parameter to Invoke-Gherkin to allow you to override the root of the search, but doing so this would basically end your ability to organize your steps at all when you need to run all your tests. You would still be able to put them in separate files, but they'd all be imported, so they would each need to be globally unique.

I think that to avoid a breaking change it should not be initialized as you did above, but instead be calculated where it is now, if no value is provided, that way, if you don't pass a parameter, each location with a feature file in it still has it's own scope for steps. For your scenarios, you'd basically always need to specify both paths as the \specs root.

The way you have it written above would always search the whole $path for both .feature and .steps.ps1 files, which would (potentially) break people who are relying on the current behavior to separate their steps files.

A third option would be to do both: allow specifying a (global) step path (perhaps even default that to $pwd\Steps but also search the sub-folders of the feature files. This might actually be the most flexible approach, since it would allow you to have conflicting steps defined while still sharing global steps. What do you think of that, @fourpastmidnight?

What I don't think I want to do is to try to allow folders in a root \Steps folder that mirror the folders in your \features folder (versus having .steps.ps1 files or \Steps folders in each feature folder).

fourpastmidnight commented 6 years ago

I think I was leaning towards what you said @Jaykul --but I'm not sure. So, let me try and state my intention again, as I fear I may have muddied things up with showing my "quick fix" code that, frankly, doesn't work as I intended.

I agree that there's the potential to break people, so yes, we have to be careful. I'm proposing the following:

Then, I propose the following heuristic:

  1. If only $FeaturePath is specified, this would pretty much work the same way today as using the $Path parameter. E.g. the specified folder(s) would be searched recursively for all *.feature files. Iff. the $StepPath parameter is not specified, then assume the root of the search for step definitions is the value provided for $FeaturePath (or the current directory, if $FeaturePath is not specified).
  2. If $StepPath is specified, use the specified path(s) to recursively search the folders at and below the specified folder(s) for all *.step.ps1 files.

What I'm proposing is that we find features and step definitions as similarly as possible to Cucumber. By default, Cucumber will search for all feature files recursively under a folder named features, unless you override it by specifying --feature (IIRC). Similarly, step definitions are found recursively under feature\step_definitions, unless you override it through another configuration option to Cucumber (I think --steps, IIRC--but it's been a long time since I've used vanilla Cucumber). The point is, there are sensible defaults (which for Pester, would be the existing behavior so as not to break people), but which can be overridden if need be.

I'm not so keen on dot-sourcing step definitions. It just feels icky somehow. But, that's just my opinion. I'm open to hearing thoughts on that.

I think this is what you were saying, but I'm not 100% sure 😉 But this makes sense to me unless I missed something above.

Lastly, I would note that I think this is less to do with global step definitions than it is about how step definitions are "scoped", and by extension, where step definitions are assumed to be found by Pester. In Cucumber, all step definitions are global, even if they're organized in folders. But Pester seems (at the present moment) to be "scoping" step definitions via their folder hierarchy, specifically, with respect to feature file folder hierarchy. This organization tends to lead to bad testing practices (at least, as described in The Cucumber Book—but this may be an artifact of the language in this case?? I'm not sure. I need to think about that more. I mean, PowerShell and its usage is very different than, say, Ruby.) I'm open to hearing more on this--however, this may all be purely philosophical while the proposal above would more than suit the interests of anyone using Pester in allowing them to organize their test suite as they see fit.

gnuechtel commented 5 years ago

@fourpastmidnight You already working on that, correctly? Will there be any pull request soon?

(Maybe you are able to look at my very simple code above? That simple stuff is working in practice ;-))

fourpastmidnight commented 5 years ago

Actually, I reread The Cucumber Book with regards to this, and I’m not sure that I was correct in my assertions in this issue. I’ve been silently mulling over this issue trying to decide if anything needs to be done.

My current thinking is that the should definitely be an option to specify the location(s) (paths) to search for step definition files. I have done a naïve implementation, but it needs more work to be production ready.

Sorry for my silence over the last week or so. I’ve been away visiting family for the holidays. I need to review the changes for your other PR, too. But I just got back home, so I’ll become more active again in a day or three 😉

Sent from my Windows 10 phone

From: Christian Gnüchtel Sent: Saturday, December 29, 2018 10:06 To: pester/Pester Cc: Craig E. Shea; Mention Subject: Re: [pester/Pester] Pester Gherkin processor assumes too much whenlocating Step files (#1129)

@fourpastmidnight You already working on that, correctly? Will there be any pull request soon? (Maybe you are able to look at my very simple code above? That simple stuff is working in practice ;-)) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gnuechtel commented 5 years ago

In my opinion the feature files should be completely separated from the step implementations. Since step definitions are technical stuff which should not distract people while working on feature files.

So I would prefer the following structure:

Features/
  Feature1/
    Feature1.system1.feature
    Feature1.system2.feature
    README.md
  Feature2/
    Feature2.system1.feature
    Feature2.system1.feature
    README.md
Sources/
  Windows/
    test/
      steps/
        Feature1.steps.ps1
        Feature2.steps.ps1

In the company I am currently working we are using such a structure with a customized version of Pester. We are also using feature files for two systems (Windows and Android) and all the test code is under Sources which contains step definitions for Pester and a Android Cucumber test project. (It is a project which only contains end-to-end tests, the production code is in another repository. But this does not really matter here, the sources folder could also contain the production code.)

With the changes I mentioned in my previous post, such a structure is possible with Pester.

I also think, that all step implementations should be global. It should not make any difference where the step files are located. Though the changes should be downward compatible (which my code hopefully does).

@fourpastmidnight Your suggestions seem very reasonable. Let's implement it 😉

fourpastmidnight commented 5 years ago

So, as much as possible, anything I do for Pester with respect to Gherkin style tests, I try to accomplish in the most “cucumber way” possible since, if you’ve used cucumber before, you won’t be surprised. As I previously started, when I went back and re-read The Cucumber Book, I think my original assertions in this issue were incorrect based on my reading into something that wasn’t there. Having said that, you should be able to specify (additional?) paths to be used to search for step definitions (I believe cucumber lets you do this). And you are correct, in cucumber, all step definitions are global, no matter where they’re defined, so long as they’re found by cucumber in the first place.

Therefore, the way I see this being implement is that Pester will use it’s default method of finding step definitions (after carefully reading The Cucumber Book, I believe Pester is working in the same way for finding step definitions), while allowing you to optionally specify additional paths to use when searching for step definitions.

In my private copy of Pester, I added the additional parameter, but I used the value of that parameter exclusively. All I need to do to implement the above is ensure the parameter value is used in addition to other paths which are searched by Pester.

Sent from my Windows 10 phone

From: Christian Gnüchtel Sent: Saturday, January 5, 2019 17:53 To: pester/Pester Cc: Craig E. Shea; Mention Subject: Re: [pester/Pester] Pester Gherkin processor assumes too much whenlocating Step files (#1129)

In my opinion the feature files should be completely separated from the step implementations. Since step definitions are technical stuff which should not distract people while working on feature files. So I would prefer the following structure: Features/

Feature1/

Feature1.system1.feature

Feature1.system2.feature

README.md

Feature2/

Feature2.system1.feature

Feature2.system1.feature

README.md

Sources/

Windows/

test/

  steps/

    Feature1.steps.ps1

    Feature2.steps.ps1

In the company I am currently working we are using such a structure with an customized version of Pester. We are also using feature files for two systems (Windows and Android) and all the test code is under Sources which contains step definitions for Pester and a Android Cucumber test project. (It is a project which only contains end-to-end tests, the production code is in another repository. But this does not really matter here, the sources folder could also contain the production code.) With the changes I mentioned in my previous post, such a structure is possible with Pester. I also think, that all step implementations should be global. It should not make any difference where the step files are located. Though the changes should be downward compatible (which my code hopefully does). @fourpastmidnight Your suggestions seem very reasonable. Let's implement it 😉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.