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

Problem matcher support for Context errors #2447

Merged
merged 1 commit into from Feb 3, 2020
Merged

Problem matcher support for Context errors #2447

merged 1 commit into from Feb 3, 2020

Conversation

tillig
Copy link
Contributor

@tillig tillig commented Jan 29, 2020

PR Summary

Fixes #2438 - allows errors that occur in Pester test context to be shown in the Problems window just like other test failures. Solution is a simple regex fix in the problem matcher for line number/location.

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
  • N/A - PR has tests
  • This PR is ready to merge and is not work in progress

I didn't see that there were any existing problem matcher tests. If there are, I'd be happy to try adding some updates to them.

@tillig
Copy link
Contributor Author

tillig commented Jan 29, 2020

For reference, I tested this on MacOS on Pester tests that have both context and test failures. The console log looks like this:

> Executing task: Invoke-Pester -PesterOption @{IncludeVSCodeMarker=$true} <

    ____            __
   / __ \___  _____/ /____  _____
  / /_/ / _ \/ ___/ __/ _ \/ ___/
 / ____/  __(__  ) /_/  __/ /
/_/    \___/____/\__/\___/_/
Pester v4.9.0
Executing all tests in '.'

Executing script /Users/tillig/dev/powershell-test/has space/Failing.Tests.ps1

  Describing Reproduce Issue

    Context Failing BeforeAll in Context block
Get-Command: /Users/tillig/dev/powershell-test/has space/Failing.Tests.ps1:9:30
Line |
   9 |              $pathToCommand = Get-Command nosuchcommand.exe | Select-O …
     |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The term 'nosuchcommand.exe' 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.

      [-] Error occurred in Context block 0ms
        At /Users/tillig/dev/powershell-test/has space/Failing.Tests.ps1:10 char:14
        The term 'othermissingcommand' 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.
    [-] has two failures - context and test 76ms
      at <ScriptBlock>, /Users/tillig/dev/powershell-test/has space/Failing.Tests.ps1: line 14
      14:         $False | Should -Be $True
      Expected $true, but got $false.

Executing script /Users/tillig/dev/powershell-test/Failing.Tests.ps1

  Describing Reproduce Issue

    Context Failing BeforeAll in Context block
Get-Command: /Users/tillig/dev/powershell-test/Failing.Tests.ps1:9:30
Line |
   9 |              $pathToCommand = Get-Command nosuchcommand.exe | Select-O …
     |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The term 'nosuchcommand.exe' 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.

      [-] Error occurred in Context block 0ms
        At /Users/tillig/dev/powershell-test/Failing.Tests.ps1:10 char:14
        The term 'othermissingcommand' 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.
    [-] has two failures - context and test 7ms
      at <ScriptBlock>, /Users/tillig/dev/powershell-test/Failing.Tests.ps1: line 14
      14:         $False | Should -Be $True
      Expected $true, but got $false.
Tests completed in 686ms
Tests Passed: 0, Failed: 4, Skipped: 0, Pending: 0, Inconclusive: 0 

The failing test fixture looks like this:

Describe "Reproduce Issue" {
    Context "Failing BeforeAll in Context block" {
        BeforeAll {
            # In a "real" scenario, this is
            # $pathToNuGet = Get-Command nuget.exe | Select-Object -ExpandProperty Source
            # &mono $pathToNuGet do-some-things-with-nuget.exe
            # to execute NuGet on a Linux/OS X box. The failure I'm seeing in the context
            # happens when mono isn't available, and possibly nuget.exe is also not there.
            $pathToCommand = Get-Command nosuchcommand.exe | Select-Object -ExpandProperty Source
            &othermissingcommand $pathToCommand
        }
    }
    It "has two failures - context and test" {
        $False | Should -Be $True
    }
}

I had two instances of this, one in the top-level folder, one in a folder called has space so I could verify spaces in filenames wouldn't mess things up.

I did have to create a tasks.json and run the Pester tests using that because the current "Run Pester tests" from the extension doesn't have the "IncludeVSCodeMarker" part.

    {
      "command": "Invoke-Pester -PesterOption @{IncludeVSCodeMarker=$true}",
      "group": {
        "isDefault": true,
        "kind": "test"
      },
      "label": "test",
      "options": {
        "cwd": "${workspaceFolder}"
      },
      "problemMatcher": "$pester",
      "type": "shell"
    }

@tillig
Copy link
Contributor Author

tillig commented Jan 29, 2020

It appears the builds are failing due to API rate limits being exceeded. I'm not sure there's anything I can do about that.

@TylerLeonhardt
Copy link
Member

Yeah that CI failure can be ignored.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for fixing this!

@nohwnd this is what we will need to change for Pester 5. We can probably just add another problemMatcher and call it pester5 instead of pester or something.

@nohwnd
Copy link
Contributor

nohwnd commented Jan 31, 2020

Cool, I think Pester v5 already prints errors like this, but I will have to check if it triggers the same problem matcher. Is there an environment variable that can be used to detect we are running in VSCode? I would like to do that rather than passing the parameter (even though it is implemented already). That would align better with modifying the output for azure devops / other build services, which is planned later.

@SeeminglyScience
Copy link
Collaborator

@nohwnd

Is there an environment variable that can be used to detect we are running in VSCode?

# Any terminal in VSCode
$env:TERM_PROGRAM -eq 'vscode'

# Specifically this extension's terminal
$env:TERM_PROGRAM -eq 'vscode' -and $psEditor

@TylerLeonhardt
Copy link
Member

Since Tasks don't run in the PS Integrated Console, you should just check the TERM_PROGRAM

@TylerLeonhardt TylerLeonhardt merged commit cc9ce31 into PowerShell:master Feb 3, 2020
@tillig tillig deleted the feature/issue-2438 branch May 12, 2020 20:23
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

Successfully merging this pull request may close these issues.

Pester test Context failures not showing up in Problems window
4 participants