Introduce a new check to upgrade user code based on API changes in Googletest.
The check finds uses of old Googletest APIs with "case" in their name and replaces them with the new APIs named with "suite".
astrelni on Jun 6 2019, 1:09 PM.Authored by
I think will be good idea to replace upgrade with modernize to be consistent with similar checks in other module.
You need to upload the entire patch, not as compared to the previous patch.
I will work on changing this in the next diff update, if your opinion holds. The context for currently using "upgrade" is:
a) The modernize module seems to contain checks that are geared at using more modern language features as opposed to API breaking changes. With some exceptions, they seem to be about changing from old to new practice while both ways remain valid code. The goal of this check is to take code that will break with future googletest API changes and turn it into working code, allowing users to upgrade the version of googletest they use. Maybe upgrade is not the best term, but I don't think modernize is a better fit.
I don't feel very strongly, just thought it would be good to provide the context. Please let me know how you'd like me to proceed.
Generally this LGTM.
Sorry, the original comment got lost between me updating two patches as I noticed I did it wrong without seeing your comment.
Unfortunately there are no versioning macros in googletest, so I'm not sure how to keep this check from providing warnings if it is run while depending on a version that is too early. The new names are in master and will be part of an upcoming version 1.9 release. I have tried to update the documentation to clarify which version of googletest this intended for. Please let me know how you think we should proceed.
I'm not fully current on what can and can't be done in clang-tidy, but i suppose if the
Yes, makes sense. Let me know what you think of the approach I've added in the latest diff. I think it is sufficient to check for the existence of one of the new macros in the right file and one new method in each matched class.
The tests as they are cover the positive case in that they would not show warning or fixes if we didn't find the replacements. The only way to test the negative is to make a second test with a second set of mock googletest headers that declare things in the v1.8 way. Is this what you would prefer?
Looks reasonable. I did not review the check itself though.
The nosuite test was a strict subset. I've combined them with a few lines that needed to be turned via preprocessor branches. Thank you, I actually wasn't aware that these tests can have multiple RUN lines.