FixStacktraceLanguage

[Akturos]

PR Summary

"Stacktrace is not filtered in non-english system languages" I found the PR #2062 and completed the approach to fix the issue. The new function get's the correct StackTracke language informations and passes them to necessary the regex filters.

PR Checklist

• [X] PR has meaningful title
• [X] Summary describes changes
• [ ] PR is ready to be merged
  • If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
• [ ] Tests are added/update (if required)
• [ ] Documentation is updated/added (if required)

[fflaten]

Thanks for looking into this. Will test and review properly later, but wanted to quickly mention this draft PR #2276. They differ in a couple ways:

• You gather language at run start vs on every call
  • Pro: Better performance 👍
• You use resource (smart!) vs throwing a real exception
  • Pro: Performance (?) and clean
  • Con: Not compatible with PSv3 and 4 which we currently support. PS v3 and v4 are end-of-life so they're not critical, but atm. Pester supports them. So maybe throwing a real exception like the linked PR is worth it for now?

It would be nice to have a way to test this during our CI to avoid breaking it in the future. Ex. by accepting an optional [string]$StackTrace parameter so we could test the regex-pattern using 2-3 language samples.

[Akturos]

Hi @fflaten

I have adapted the code according to your wishes and added a fallback scenario for all versions to the regex of PR #2276. I also separated this function and added a parameter [string]$StackTrace which allows you to test this function. But after the commit there was a bug in PS6_2

https://github.com/pester/Pester/blob/7ca9c814cf32334303f7c506beaa6b1541554973/tst/Pester.RSpec.Demo.ts.ps1#L132C3-L132C68

which points to a runtime error. Can you please run the failed tests again, I think it's probably a onetime timing error.

The tests are successfully complete now.. so everything is OK

[nohwnd]

@Akturos will you address that PR remarks, or should I finish it up so we can ship it? :)

[Akturos]

@nohwnd

When modifying the code, I noticed that without throwing a exception for PS version 3/4, the necessary values for the stacktracelanguage can no longer be read from the self-generated error. This raises the question of whether you really want it that way, as the error can also be used to generate the appropriate stacktracelanguage for PS version 3/4. If the error is removed, this will no longer work and only English will be the default for PS version 3 / 4.

I have adapted the variables to the default notation.

Regarding the test. I have tried to build a test with the cultures. However, this is not possible, or I have not managed to set the cultures in the PS session in such a way that I could use them for a test.

& { $stackTraceRessourceName = 'DebuggerStrings' [System.Threading.Thread]::CurrentThread.CurrentCulture = 'fr-FR' [System.Threading.Thread]::CurrentThread.CurrentUICulture = 'fr-FR' $r = [System.Resources.ResourceManager]::new($stackTraceRessourceName, [System.Reflection.Assembly]::GetAssembly([System.Management.Automation.Breakpoint])) $r.GetString('StackTraceFormat') } Maybe you have a better idea.

Cheers Acturos

[nohwnd]

When modifying the code, I noticed that without throwing a exception for PS version 3/4, the necessary values for the stacktracelanguage can no longer be read from the self-generated error.

We can probably throw once and get the elements from that? And then use normal control flow? Extensions are not super expensive, on my system it is 30ms to throw and catch 1000 of them, still it is IMHO unnecessary to throw and catch on every assertion fail.

[fflaten]

We can probably throw once and get the elements from that?

Isn't it executed once during import and cached in a module variable?

Personal opinion:

• Change to once per run in case it's possible to adjust culture in current process?
• I'd refactor control flow to return early instead of nested if/else try/catch.

[claudiospizzi]

Hi all

I agree with @fflaten - the Idea was to store the stack trace language once in module scoped variable $Script:stackTraceLanguage. This is done in the script Pester.Runtime.ps1, which is loaded once during module import, right?

Throwing an exception once and only for rare cases (PSVersion < 5 or if there is any unexpected issue with the resource manager) sounds acceptable.

Would be cool, if we can merge that to have a better experience for non-English systems.

Thank you.

[Akturos]

Hi all I also agree with @fflaten. I have reset the branch to the commit ce24250aa54e9236879d873b7af868c6b4e3a272. I think this is the most useful solution. I have also adjusted the variables.

It would be great if we could ship this to prdouction

[Akturos]

Happy new year all.

Is there an update from your side guys? Should anything else be adjusted or can we ship it. Thanks for your feedback.

[fflaten]

Happy new year! 🙂 I've added a review with a few change requests.

I think nohwnd's a bit busy, so final approval + merge might take a little time.

[nohwnd]

I am here now and will be for few more weeks at least, so best time to resume work on this :))

[nohwnd]

Please figure out a way to test this. Changing current thread culture did not affect stack trace for me. And I don't think the proposed code was working because the regex was rewritten few lines below by the if ($true) code. So I would like a way to ensure this works and keeps working.

[System.Threading.Thread]::CurrentThread.CurrentCulture = [System.Globalization.CultureInfo]::GetCultureInfo("cs-cz")
[System.Threading.Thread]::CurrentThread.CurrentUiCulture = [System.Globalization.CultureInfo]::GetCultureInfo("cs-cz")

1.5
function a {
    try { throw "a" } catch { $_.ScriptStackTrace }
}

[nohwnd]

I know what I broke in the code, the # PESTER BUILD is a build directive that emits that code only when we are building locally without inlining all the scripts into single pester.psm1 file. I forgot about that, and thought the code always runs because of it if($true). It is probably better to use that pattern of removing all in the folder, because there is also pester.ps1 that is not filtered now.

[nohwnd]

I would love to fix, but we don't have a repro for the original problem so it looks like it is not a problem anymore. If someone finds out how to repro, or even better, test this. We can re-open.