vscode-cmake-tools
vscode-cmake-tools copied to clipboard
Search CTest output for failure locations (#4418)
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 the DEF_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
@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?
Could we use a more strict type than
Objectwhen defining the settings/emitters?
Fixed. I tightened up the corresponding schema in package.json while I was at it.
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 TestMessage.actual and .expected were filled in, that I don't think I want to match them anyway.
The regexp for GoogleTest/GoogleMock is (.+?):((?:\\d|#)+): *(?:[Ff]ailure|[Ee]rror)\n+((?:.+\n)*), and it is able to match multiple failures in the same test if they are reported:
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?
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.
@gcampbell-msft just rebased this against the latest post-1.21.36 main. The only outstanding question I think is whether you want me to make cmake.ctest.failurePatterns an empty array instead of being pre-populated with some generic regexps. Other than that I think it's ready for merging if you do.