Updated Pester Problem Matcher #1362
Conversation
…e [-] from the message
|
This resolves #1291. |
…emove ms from message.
|
Here are a few screen shots of this working. Pester Output Problem Matcher |
|
Nice catch! Thanks. |
package.json
Outdated
| @@ -176,7 +176,7 @@ | |||
| "severity": "error", | |||
| "pattern": [ | |||
| { | |||
| "regexp": "^\\s*(\\[-\\]\\s*.*?)(\\d+)ms\\s*$", | |||
| "regexp": "^\\s*(?:\\[-\\]\\s)(\\s*.*?)(?:\\s\\d*\\.?\\d*s|\\s\\d*\\.?\\d*ms)\\s*$", | |||
There was a problem hiding this comment.
I wonder if we could simplify this a bit to:
"^\\s*(?:\\[-\\]\\s+)(.*?)(?:\\s+\\d+\\.?\\d*\\s*m?s)\\s*$"
I don't think we want to include any spaces at the beginning of the message (capture group 1), best to have the first non-capture group scoop up all the spaces after the [-]. Ditto with the second non-capture group - let's have it scoop up all the space at the end of the message.
Also, shouldn't we expect at least one digit (\d+) before the required s or ms? Finally, how about we add a \s* between the time and the units in case they ever decide that the display looks better with a space in there. :-)
There was a problem hiding this comment.
I like it. Great feedback. I'm not great with regular expressions (said everyone on the internet), so your write up was a great learning experience for me. I'll get it updated.
…2, made at least one digit required for time elapsed, and put an optional space after the time elapsed and units.
|
One last request - did you test the updated regex against Pester? Also, what version of Pester are you using? I did some basic testing against faked output strings. :-) |
|
I threw the same regular expression in my project's tasks.json and it works properly in there for both milliseconds and seconds. I'm running with Pester 4.3.1. See attached screen shots.
|
|
@awickham10 Merged. Thanks for your contribution! |
|
No problem - glad to help! |
|
Nice work awickham10, I rarely read release notes but just happen to see your user name attached to this! |
|
Thanks @derbium! Need to catch up soon. |
PR Summary
Updated the Pester problem matcher. Resolves #1291.
PR Checklist