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

Detailed Output vs Test Report - Skipped #2220

Closed
3 tasks done
Splaxi opened this issue Jul 21, 2022 · 6 comments · Fixed by #2254
Closed
3 tasks done

Detailed Output vs Test Report - Skipped #2220

Splaxi opened this issue Jul 21, 2022 · 6 comments · Fixed by #2254

Comments

@Splaxi
Copy link

Splaxi commented Jul 21, 2022

Checklist

What is the issue?

It seems that the logic with expanding the path of a given test, is being handled differently when we are talking about the detailed output and the output object.

BeforeDiscovery {
    $colLogicApps = @(
        @{ Name = "1" }
        @{ Name = "2" }
        @{ Name = "3" }
        @{ Name = "4" }
    )
}

Describe "Execute <name>" -ForEach $colLogicApps {
    BeforeEach {
        if ($Name -eq "3") {
            Set-ItResult -Skipped -Because "Just because"
        }
    }

    It " Skip when 2" {
        if ($Name -eq "2") {
            Set-ItResult -Skipped -Because "Just because"
        }

        1 + 1 | Should -be 2
    }
}

The detailed output expands the path of each test / describe:

image

But the output object looks like this:

$temp = Invoke-Pester -Path "..." -Output Detailed -PassThru
$temp.tests | ft Name, Path, ExpandedPath

image

Expected Behavior

I would expect the expanded path to work the same way as the detailed output, so when you are generating NUnit/JUnit files, the would reflect the same values.

Steps To Reproduce

No response

Describe your environment

No response

Possible Solution?

No response

@fflaten
Copy link
Collaborator

fflaten commented Jul 22, 2022

Thanks for the report. ExpandedName and ExpandedPath are evaluated after the setups because they could create variables used in the name iirc. So skipping inside BeforeEach vs It will be different since the latter will already be finished with setups -> name is expanded.

We could however update the placeholder-value for ExpandedPath that you see now to be expanded as far as we reached before skip/failure. That would solve the sample in this issue where only the block has a variable + make it easier to identify the correct block in general. 👍

@Splaxi
Copy link
Author

Splaxi commented Jul 22, 2022

Thanks for the feedback.

Is there any way I can assist with this, so we can have the change to be part of the next release? I'm new to the code base of pester, but I'm up for the challenge to see if I can fix it and do the PR.

Just need some pointers, then I can test local and see if my scenario can be handled or not.

@fflaten
Copy link
Collaborator

fflaten commented Jul 22, 2022

Absolutely, contributions are always welcome 😃
You can see how to build and test in CONTRIBUTING.md. I personally use the devcontainer in the repo so I don't need .NET locally.

As for the code:

  • Test (It)
    You can see how ExpandedPath is set here. I'm thinking we could set it once more earlier in Invoke-TestItem before the skip-check and/or framework-setup. $Test should be the same as CurrentTest at that point.

  • Block (Describe/Context)
    Current logic is here. ExpandedPath could be set the same way probably around L321.

  • We should also add a test or two similar to this to verify that it works when a) a block fails due to BeforeAll, b) a test with -Skip or that fails (or Set-ItResult which is a special exception) in BeforeEach. These are P-test which are used to test behavior of Pester-engine itself from the outside. b is like describe/context, i is like it and we can only use the Verify-* assertions found in the tst\axiom-folder.

@fflaten
Copy link
Collaborator

fflaten commented Oct 31, 2022

Still interested in submitting a PR @Splaxi? 🙂

@Splaxi
Copy link
Author

Splaxi commented Oct 31, 2022

Yeah - but when I tried the last time, I couldn't find a way to solve the issue.

So I feel that I'm smart enough to tackle the challenge, regardless of me having the issue 🤦‍♂️

@fflaten
Copy link
Collaborator

fflaten commented Nov 1, 2022

Looking back at it months later - it wasn't my best explanation 😄
I think I've covered it in the related PR.

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 a pull request may close this issue.

2 participants