PowerShellEditorServices: 'Run tests|Debug tests' Pester code lens does not show when Describe block is interpolated string

@rkeithhill it does not, yet. If I have some time I will code it for 4.5.1 over the weekend

I a currently thinking that it might be best for the moment as a short term solution to change it back to double quotes because it will work for most cases if the initialization code has run once.

We had a specific bug we fixed by changing to single quotes. I’d rather not have a regression in this case. It may be necessary to provide info from PSES back to the extension indicating whether we have string constant expr in which case we use single quotes or an expandable string in which case we could use double quotes. But again, as @SeeminglyScience and I came to the conclusion that in likely many cases the variable required to be interpolated will not be defined in the PS integrated console at the time of the first run and maybe even not for subsequent runs. I think having a feature that partialy works (in certain scenarios) does not make for a good user experience. All in all, I think it is better to stick with single quoting and skip attempting to handle expandable strings - other then to display an error in VSCode when the associated code lens is invoked.

@rkeithhill if it doesn’t, we can just still show the codelens and have it display an error when clicked if there’s no way to, on the fly, change it to an error

1. We don’t add CodeLens to Describes with variables in them

I’d say do add the code lens, but write a descriptive error message. A rule works too I suppose but that’s more default stuff folks would need to suppress.

From what I gather here (and @nohwnd please confirm if you haven’t already) having parameters inside of the string is not a best practice since test strings should be static.

This helps make tests less cryptic and also helps us out because we have a best practice to depend on. This also means that we (the extension) can help enforce the best practice.

What if we do this:

1. Add a Run all tests CodeLens to the top of the file. This helps @bergmeister with a button that will run all the tests.

2. We don’t add CodeLens to Describes with variables in them

3. We add a PSSA rule that warns to “not have variables in test names”

We enforce a best practice, but still give users the ability to click a button to run tests

@bergmeister do you want to try implementing this so it filters on path+line? The implementation in Pester should be very simple (plus you already know the codebase), and you’d get some practice in TS. I’d accept a PR for it.

Playing with this a bit more:

$duck = 'Hello'

Describe "My test $duck" {
    It "works" {
        1 | Should -Be 1
    }
}

There’s no way to execute the test with Invoke-Pester ./My.Tests.ps1 -TestName <name> here without knowing what the value of $duck is statically. To make Pester execute it, we need Invoke-Pester ./My.Tests.ps1 -TestName 'My test Hello'. Since the CodeLens execute button is executed outside of the file, we can’t just ever make it work in general – and I think it’s worth noting that this is a limitation that exists for Pester without our CodeLens too (not saying it should work, just that the problem is one of expecting runtime information to be known statically).

Circling back to the original problem statement:

I sometimes include test details in the describe block that are sometimes only known at runtime

I think “only known at runtime” defines this problem as out of scope for something based on AST analysis. If it’s only known at runtime, then there’s no way for us to ask Pester to execute this.

Yes, there is v5-alpha1 since yesterday that can discover tests and filter them by It, and only run appropriate setups and teardowns, so it should be ideal for running tests selectively.

It already can filter by path, you can see me do someting very similar with re-running failed tests here.

At the moment those published functions do not take the result object + paths (to keep the api smaller and simpler), instead test discovery is done every time from scratch.

But internally the limitation is not there, if I add functions that clean a dirty discover/run result, it could easily take a cached discovery object and run with different filters. Which is useful not only for this scenario, but for example if you want to observe filesystem, discover tests in a single file, join that discovery result to your cached results and re-run all previously failed tests in all files. Or when you have a huge test suite where discovery takes signifcant amount of time, and multiple configurations to run it with.

I’m still very uneasy about the idea of some of the code lenses running all tests and some running only the nearest describe block. I’d much rather see a descriptive error message explaining that the test name should be static.

It’s probably not a huge deal for just describes, but ideally we’d support a code lens for every it block. I believe @nohwnd mentioned filtering by it block names is potentially coming in the next major release.

Interestingly the single quote is by design, which this issue documents as it used to be double quotes but was not working for interpolated strings in special scenarios where ToString() was resulting in something that Pester could not understand, maybe this is also an issue with Pester… Maybe the better solution is to just run Invoke-Pester without the -TestName (i.e. on the whole file) if the Describe string contains a dollar sign

What if we created the command line using double quotes e.g. -PesterOption @{IncludeVSCodeMarker=$true} -TestName "foo on PSVersion $version"? Or maybe, if we can pull the string from the Describe block, it would be better to use that string quoting. That way if you used single quotes to prevent interpolation, we’d honor that.