-
Notifications
You must be signed in to change notification settings - Fork 481
Search CTest output for failure locations (#4418) #4420
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
base: main
Are you sure you want to change the base?
Conversation
@malsyned I think that this is a useful PR and is an interesting additive feature. There is also minimal risk for regression, as by default this will not perform any additional computation because the default is without this. Before spending time reviewing, could you fix the merge conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use a more strict type than Object
when defining the settings/emitters?
Fixed. I tightened up the corresponding schema in |
Adds configuration setting `cmake.ctest.failurePatterns`, a list of regular expressions and capture group indices for matching file, line, message, expected output, and actual output from failing tests. Pattern matches are used to populate `TestMessage` objects with more specific information than is available through the `DEF_SOURCE_LINE` property.
The string passed into `searchOutputForFailure()` has been normalized to CRLF, so make sure the substrings produced from it also have CRLF line endings. https://code.visualstudio.com/api/extension-guides/testing#test-output
The package.json schema for `cmake.ctest.failurePattens` was more lenient in some ways than the corresponding `FailurePatternsConfig` TypeScript type. Bring them in line with each other.
ce9c88f
to
58a6a64
Compare
|
||
const testMessage = new vscode.TestMessage(normalizeCRLF(message)); | ||
testMessage.location = new vscode.Location( | ||
vscode.Uri.file(file), new vscode.Position(line, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcampbell-msft I have a small hesitation about this line, I would love for you to weigh in on it. It's assuming that the output file name matched by the regex is an absolute path suitable for passing to Uri.file()
. It always has been in my testing, and I think due to the fact that CMake tends to generate build systems that operate exclusively on absolute paths, I'm guessing it's likely that it always will.
However, if you think otherwise, and you think there's a reasonable directory to resolve relative paths against, I would be happy to add something to this PR.
This change addresses item #4418
This changes visible behavior
The following changes are proposed:
Adds configuration setting
cmake.ctest.failurePatterns
, a list of regular expressions and capture group indices for matching file, line, message, expected output, and actual output from failing tests.The purpose of this change
Populate
TestMessage
objects with more specific information about test failures than is available through theDEF_SOURCE_LINE
property.Other Notes/Information
I based the design of this feature around the design of Task Problem Matchers. It lacks some of the advanced features of Problem Matchers, like named patterns, pattern inheritance, multi-regexp patterns, and loops. I'm happy to add those to the implementation if needed, but I thought it better to start with the simplest thing that could work and grow it in response to feedback.
Screenshots