Add 'Run/Debug Pester tests' command and file tab menu #1698

bergmeister · 2019-01-13T22:55:18Z

PR Summary

Implements #1667 by adding a command and file tab menu to run all Pester tests on a file. The file tab menu will only appear for .tests.ps1 files (using a case insensitive comparison).
I tested this on Windows 1809 using vscode and the latest vscode-insiders when using Windows PowerShell 5.1. I also made sure that the code lens being used by PSES does not get broken, hence why new commands were added rather than trying to adapt for the different kind of uri in the launchTests function. Here is a zip of my locally built vsix for testing: PowerShell-insiders.zip

New file tab menu entries:

Commands:

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

PR has a meaningful title
Summarized changes
PR has tests
This PR is ready to merge and is not work in progress
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

rkeithhill · 2019-01-14T01:33:20Z

package.json

@@ -233,6 +238,12 @@
          "group": "2_powershell"
        }
      ],
+      "editor/title/context": [
+        {
+          "when": "resourceExtname  == .ps1",


Does this work:

"when": "resourceFilename =~ /\.[Tt]ests\.ps1$/"

Thanks, I had to escape the backslash in JSON and then your suggestion worked: "resourceFilename =~ /\\.[Tt]ests\\.ps1$/"
The PR is now ready for review :)

rkeithhill

LGTM. Thinking a bit on the case-insensitivity - maybe it would be better to use /\\.tests\\.ps1$/i?

rkeithhill · 2019-01-14T18:38:26Z

You know, we could also add a PowerShell.DebugPesterTests or PowerShell.DebugPesterTestsFile command.

bergmeister · 2019-01-14T18:49:15Z

@rkeithhill Thanks for the tip with the completely case insensitive comparison for the tests.ps1 file extension, I tested it and it works :)
Yes, I was thinking about adding a Debug Pester tests command/menu as well and when I was testing it again, I realised that the current implementation is actually Debug Pester tests due to the defaults in PesterTests.ts. I guess I'll have to pass in args but maybe the command should rather be split I guess? I'll therefore make it WIP again

rkeithhill · 2019-01-14T21:16:14Z

Just as with your original post, I was thinking that we could have two functions RunPesterTestsFile() and DebugPesterTestsFile() that internally called RunPesterTests with the appropriate arguments. Then we'd need to register those commands in the PesterTests.ps1 file.

bergmeister · 2019-01-14T22:10:43Z

Yes, I was even thinking of just changing to make RunPesterTests just run the tests without the debugger and create a new function DebugPesterTests (internally both would lookup the file name if it is null and use the common launchTests function), this way we can avoid creating an additional layer of wrapping that is not necessary. The downside of that is that it requires a change in PSES which calls into RunPesterTests at the moment. Give me a bit more time to play with this but having a WIP has definitely helped to discuss the issues and possible solutions together :)

rkeithhill · 2019-01-14T23:43:06Z

Keep in mind that we are sharing this command with the code lens request message we get from PSES.

…ent file uri. Add support for debugging as well

into PesterFileTab

bergmeister · 2019-01-16T17:56:37Z

@rkeithhill Yes, after playing a bit with it, it turned out to be easier to add new commands as you suggested instead of trying to correct the uri logic inside launchTests. I also added support for Debug Pester tests and made sure that Run Pester tests does not attach the debugger any more. The PR is now ready for review again and I updated the description

rjmholt · 2019-01-16T18:44:06Z

src/features/PesterTests.ts

+        this.command = vscode.commands.registerCommand(
+            "PowerShell.RunPesterTestsFromFile",
+            (uriString, runInDebugger, describeBlockName?) => {
+                this.launchTests(vscode.window.activeTextEditor.document.uri, false, describeBlockName);


Maybe worth documenting the true and false somehow? I assume it's "start debugger" or similar?

It's the runInDebugger parameter, which is defined on launchTests, isn't that sufficient/self-documenting? I simplified and minimised the lambda expression, maybe it is clearer now? As far as I've seen, TypeScript does not support named parameters.

…hitespace change

rkeithhill · 2019-01-17T01:59:04Z

src/features/PesterTests.ts

@@ -17,13 +17,13 @@ export class PesterTestsFeature implements IFeature {
    constructor(private sessionManager: SessionManager) {
        this.command = vscode.commands.registerCommand(
            "PowerShell.RunPesterTestsFromFile",
-            (uriString, runInDebugger, describeBlockName?) => {
-                this.launchTests(vscode.window.activeTextEditor.document.uri, false, describeBlockName);
+            () => {


Thanks! I was just going to suggest this but see you've already done it. :-)

add run pester tests command and file tab menu

1f9959d

rkeithhill reviewed Jan 14, 2019

View reviewed changes

only add menu for tests.ps1 or Tests.ps1 files

fe41d7b

bergmeister changed the title ~~WIP Add 'Run Pester tests' command and file tab menu~~ Add 'Run Pester tests' command and file tab menu Jan 14, 2019

rkeithhill approved these changes Jan 14, 2019

View reviewed changes

make check for tests.ps1 completely case insensitive

08b8bb5

bergmeister changed the title ~~Add 'Run Pester tests' command and file tab menu~~ WIP Add 'Run Pester tests' command and file tab menu Jan 14, 2019

bergmeister changed the title ~~WIP Add 'Run Pester tests' command and file tab menu~~ WIP Add 'Run/Debug Pester tests' command and file tab menu Jan 14, 2019

bergmeister added 2 commits January 16, 2019 17:50

add new commands to separate concerns due to PSES usage with a differ…

fb0760e

…ent file uri. Add support for debugging as well

Merge branch 'master' of https://github.com/PowerShell/vscode-powershell

b8f1990

into PesterFileTab

bergmeister changed the title ~~WIP Add 'Run/Debug Pester tests' command and file tab menu~~ Add 'Run/Debug Pester tests' command and file tab menu Jan 16, 2019

rjmholt approved these changes Jan 16, 2019

View reviewed changes

simplify lambda syntax to minimise parameters and revert accidental w…

969fa92

…hitespace change

rkeithhill reviewed Jan 17, 2019

View reviewed changes

rkeithhill merged commit 68b373f into PowerShell:master Jan 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'Run/Debug Pester tests' command and file tab menu #1698

Add 'Run/Debug Pester tests' command and file tab menu #1698

bergmeister commented Jan 13, 2019 •

edited

Loading

rkeithhill Jan 14, 2019

bergmeister Jan 14, 2019 •

edited

Loading

rkeithhill left a comment

rkeithhill commented Jan 14, 2019 •

edited

Loading

bergmeister commented Jan 14, 2019

rkeithhill commented Jan 14, 2019

bergmeister commented Jan 14, 2019 •

edited

Loading

rkeithhill commented Jan 14, 2019

bergmeister commented Jan 16, 2019 •

edited

Loading

rjmholt Jan 16, 2019

bergmeister Jan 16, 2019 •

edited

Loading

rkeithhill Jan 17, 2019

Add 'Run/Debug Pester tests' command and file tab menu #1698

Add 'Run/Debug Pester tests' command and file tab menu #1698

Conversation

bergmeister commented Jan 13, 2019 • edited Loading

PR Summary

PR Checklist

rkeithhill Jan 14, 2019

Choose a reason for hiding this comment

bergmeister Jan 14, 2019 • edited Loading

Choose a reason for hiding this comment

rkeithhill left a comment

Choose a reason for hiding this comment

rkeithhill commented Jan 14, 2019 • edited Loading

bergmeister commented Jan 14, 2019

rkeithhill commented Jan 14, 2019

bergmeister commented Jan 14, 2019 • edited Loading

rkeithhill commented Jan 14, 2019

bergmeister commented Jan 16, 2019 • edited Loading

rjmholt Jan 16, 2019

Choose a reason for hiding this comment

bergmeister Jan 16, 2019 • edited Loading

Choose a reason for hiding this comment

rkeithhill Jan 17, 2019

Choose a reason for hiding this comment

bergmeister commented Jan 13, 2019 •

edited

Loading

bergmeister Jan 14, 2019 •

edited

Loading

rkeithhill commented Jan 14, 2019 •

edited

Loading

bergmeister commented Jan 14, 2019 •

edited

Loading

bergmeister commented Jan 16, 2019 •

edited

Loading

bergmeister Jan 16, 2019 •

edited

Loading