Page MenuHomePhabricator

[clang-format][WIP] Run more stability FormatTests
AcceptedPublic

Authored by JakeMerdichAMD on May 25 2020, 11:16 AM.

Details

Summary

I've been seeing a lot of complaints of unstable output in the bug
tracker, and noticed that a good chunk of our tests weren't checking
this. The verifyFormat functions do this, but a lot of tests don't use
these, and they miss out on the stability checks.

This change replaces all instances of EXPECT_EQ(..., format(...)) with a
verifyFormat equivalent (I'll share the regex if you like; it's
horrifying). It also narrows the criteria to rerun a test with
objective-c in verifyFormat, since some of the migrated tests were
(correctly) failing when run with ObjC vs Cpp.

WIP because:

  1. Several tests are failing because their expected output is unstable, and the underlying issues there need to be fixed.
  2. It's harder to differenciate the expected vs input text than using EXPECT_EQ. Ideas?
  3. This only does it for FormatTests.cpp. Needs to be done on the rest.

I wanted to get some feedback and general opinions on this as I debug
the underlying issues, as well as to encourage any new tests to be
written with verifyFormat instead of EXPECT_EQ. Thoughts?

Diff Detail

Unit TestsFailed

TimeTest
40 msClang-Unit.Format/_/FormatTests::Unknown Unit Message ("")
Note: Google Test filter = FormatTest.BreaksStringLiterals [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
10 msClang-Unit.Format/_/FormatTests::Unknown Unit Message ("")
Note: Google Test filter = FormatTest.BreaksStringLiteralsWithTabs [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
20 msClang-Unit.Format/_/FormatTests::Unknown Unit Message ("")
Note: Google Test filter = FormatTest.BreaksStringLiteralsWithin_TMacro [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
20 msClang-Unit.Format/_/FormatTests::Unknown Unit Message ("")
Note: Google Test filter = FormatTest.BreaksWideAndNSStringLiterals [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
30 msClang-Unit.Format/_/FormatTests::Unknown Unit Message ("")
Note: Google Test filter = FormatTest.DoNotBreakStringLiteralsInEscapeSequence [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
View Full Test Results (17 Failed)

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2020, 11:16 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
JakeMerdichAMD added a project: Restricted Project.May 25 2020, 11:17 AM
MyDeveloperDay accepted this revision.May 25 2020, 12:27 PM

I think this is a noble effort, assuming all the tests pass as far as I'm concerned this LGTM.

How about committing all the ones that pass, then work on the others that don't separately.

This revision is now accepted and ready to land.May 25 2020, 12:27 PM