Search CTest output for failure locations (#4418) #4420
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? |
gcampbell-msft
left a comment
gcampbell-msft
left a comment
There was a problem hiding this comment.
Could we use a more strict type than Object when defining the settings/emitters?
Fixed. I tightened up the corresponding schema in |
malsyned
force-pushed
the
failure-patterns
branch
from
ce9c88f to
58a6a64
Compare
|
I have been doing some work to make sure that this feature is powerful enough to match GoogleTest and GoogleMock failure patterns. I think it is. I went in thinking it might be challenging to match the fairly complicated set of expected vs. actual patterns, but actually, GoogleTest and GoogleMock provide so much additional info in their messages that would be hidden if The regexp for GoogleTest/GoogleMock is So, that's increased my confidence in this with its current feature set. |
|
@paulmaybee @gcampbell-msft Is there anything else you all would like on this PR before you'd consider merging it? |
gcampbell-msft
left a comment
gcampbell-msft
left a comment
There was a problem hiding this comment.
I think this is a cool feature! It seems to present very little risk for regressions of other features and is purely additive, so I think this looks good, and it seems you have done good testing.
I left just a comment or two, but overall I think this looks great!
I'm glad you like it! Thanks for the review. I'll work on your comments today. |
malsyned
force-pushed
the
failure-patterns
branch
from
4889b91 to
bbb0604
Compare
malsyned
force-pushed
the
failure-patterns
branch
from
bbb0604 to
c63f62f
Compare
|
@gcampbell-msft just rebased this against the latest post-1.21.36 |
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.
hanniavalera
force-pushed
the
failure-patterns
branch
from
c63f62f to
70afc62
Compare
hanniavalera
left a comment
hanniavalera
left a comment
There was a problem hiding this comment.
@malsyned Thanks for getting this done! As to your question, I think you leaving the default patterns makes sense as it improves the user experience from the start! The patterns you've added look generic enough to work with standard error output formats without causing issues.
4 participants
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
TestMessageobjects with more specific information about test failures than is available through theDEF_SOURCE_LINEproperty.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