[RFC] Disable clang-format in the clang/test tree

I propose that we disable clang-format entirely in the clang/test tree (see D128706). For greater visibility, I’m opening an RFC for this change in addition to the Phabricator review.

Today, clang/test contains a .clang-format file that sets a column limit of 0 as well as the AlwaysBreakTemplateDeclarations option. The intent seems to be to make clang-format do something that’s appropriate for test input files.

However:

  • In practice, it seems it’s hard to make clang-format do exactly what we want in tests; see the back-and-forth between 13316a7 and 7b5bddf.

  • Some tests require specific formatting that deviates from the style guide for the purposes of the test. It would be possible to surround these tests in clang-format off and clang-format on, but this hasn’t historically been done.

  • Many existing tests aren’t formatted the way clang-format would format them. When people make changes to these tests, this causes spurious CI failures that can drown out real failures and cause a tendency to ignore failing CI runs. We could make a bulk change that clang-formats all of the tests, but it would likely not be feasible to manually check all of the changes, so we would risk introducing formatting that is suboptimal for the test (and tests have specific formatting requirements that production code does not).

See also bug #55982.

6 Likes

I’m fully in support of this. As a code reviewer, it’s been a near-constant distraction with the precommit CI pipeline (specifically the Linux one) claiming a failure due to clang-format behavior in tests. Given that we have tests that cannot be formatted, and that we’ve never really requested that tests be formatted the same as other sources, I think disabling clang-format checks in all test directories is a good approach. I’m most worried about Clang’s tests because that’s what I personally run into most often, but I don’t see a reason we’d want any test directories to require formatting (aside from, you know, the tests specific to testing formatting).

1 Like

I also support this, I’ve ran into the issue many times -especially in tests featuring purposefully invalid grammar constructs, clang format will break the validity of these tests.
It would be useful to still check for ends of lines and trailing whitespaces, but I’m not sure how difficult this would be

+1. I also met some similar failures but I don’t think it makes a lot of sense.

At the request of @martinboehme (who will be out for a while), I’ve taken over the patch. Given that there’s not been any dissent to the idea in this RFC in the past week and the thread seems to have wound down, I’m likely to push the changes sometime later this week unless I hear otherwise.

The changes for this RFC were landed in fee77a20732cb629629c52123643fa5928ebfc1c, thanks all!