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

Fix issue pester#2062 "Stacktrace is not filtered in non-english system languages" #2276

Closed
wants to merge 3 commits into from

Conversation

Nacos
Copy link

@Nacos Nacos commented Dec 15, 2022

PR Summary

Fix issue #2062 "Stacktrace is not filtered in non-english system languages"

When using StackTraceverbosity = 'Filtered', pester internals are filtered from the stack trace using regex matching.
However, Windows stack trace are localized depending on the language of the OS.
The words "At" and "Line" which can be translated (or not) depending on the language of the system.

This PR introduce an heuristic-like solution in ConvertTo-FailureLines function :

We throw an exception on purpose, then catch it in order to detect the localized strings of the stack trace.
We then feed these localized strings back to the regex for filtering pester internals.

PR Checklist

  • PR has meaningful title
  • 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)

…em languages"

Pester internal lines are filtered from Stack Traces using regex.
Windows stack traces are localized depending on the language of the OS.

In ConvertTo-FailureLines, we throw an exception on purpose, then catch it
in order to detect the localized strings of the stack trace.
We then feed these localized strings to the regex for filtering.
@Nacos Nacos marked this pull request as draft December 15, 2022 14:46
@Nacos
Copy link
Author

Nacos commented Dec 15, 2022

FYI, this is a work in progress.

I do not know if this is the right approach to fix this issue.
Would love some feedback about my proposed change.

Also, i think building the module without -Inline will not work because of the code at lines 540-548

@fflaten
Copy link
Collaborator

fflaten commented Dec 18, 2022

Thanks for looking into this!

Last year I intended to simply replace at|line with \S+ as I assume all languages use a single word and the same placements. Unfortunately I had some issues getting multiple samples last year to verify that assumption and include as tests.

While this PR is clever and more precise, it follows the same assumptions. I wonder if it might be unnecessary complex?

A bonus with your approach is that we could technically localize the added tracelines for PesterAssertionFailed, though I'm not sure if we should since the translation might not be great.

Also, i think building the module without -Inline will not work because of the code at lines 540-548

I don't follow. Wouldn't the same word-replacements apply there as well? Could you elaborate?

- update $isPesterFunction variable wrapped inside PESTER_BUILD
so that building Pester without -Build will work with the new behavior
@Nacos
Copy link
Author

Nacos commented Dec 20, 2022

Hi,

Last year I intended to simply replace at|line with \S+ as I assume all languages use a single word and the same placements. Unfortunately I had some issues getting multiple samples last year to verify that assumption and include as tests.

While this PR is clever and more precise, it follows the same assumptions. I wonder if it might be unnecessary complex?

From my experience, localization of technical messages (such as Stack Traces) on Windows can be quite inconsistent depending on the language/culture of the system.
And it's not only applied to words but also to punctuation according to local conventions.

For example, on a French(France) machine, the ":" is preceded by a non-breakable space...
(That's why I used \s* when trying to extract the Filename with the regex).

I may be able to gather stack traces for a few different languages and share them here if that can be useful to you.

Also, i think building the module without -Inline will not work because of the code at lines 540-548

I don't follow. Wouldn't the same word-replacements apply there as well? Could you elaborate?

I have pushed a new commit to my branch that modifies the code inside these lines.
However, unlike the code at lines 556-557, this if($true) "PESTER_BUILD" block does not contains special code related to non-Windows systems.

I currently don't have the means to test on non-Windows system so my concern may be a non-issue though.

@fflaten
Copy link
Collaborator

fflaten commented Dec 20, 2022

👍

I may be able to gather stack traces for a few different languages and share them here if that can be useful to you.

Would be nice to have as part of tests.

this if($true) "PESTER_BUILD" block does not contains special code related to non-Windows systems.

No need. That part uses the folderpath in the pattern, so will always have the correct slash/backslash. 🙂

@fflaten fflaten mentioned this pull request Sep 27, 2023
5 tasks
@nohwnd
Copy link
Member

nohwnd commented Apr 12, 2024

There is another PR #2391 implementing this in hopefully the most reliable way, and we cannot repro the issue. If you know how to make the stack traces show up in different language or how to test this on en-us system that would be awesome contribution to the other PR.

@nohwnd nohwnd closed this Apr 12, 2024
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.

3 participants