[PATCH] D125946: Handles failing driver tests of clang

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/test/Interpreter/clangtests.cpp:1
+// RUN: clang-repl %S/../Lexer/badstring_in_if0.c -Xcc -E -Xcc -verify
+// RUN: clang-repl %S/../Lexer/unknown-char.c -Xcc -E -Xcc -verify
----------------
v.g.vassilev wrote:
> dblaikie wrote:
> > dblaikie wrote:
> > > v.g.vassilev wrote:
> > > > @rsmith, would it be acceptable to have a test that refers other tests from the repo in that manner?
> > > Generally that's not done - and the inputs should be moved into an "Inputs" subdirectory and shared from there. Tests that are in different subdirectories - not sure if there's a good way to share those, maybe with an "Inputs" directory in a parent directory of both tests? We might not have precednt for that
> > But more broadly, could you explain what the goal of these tests are? Generally I would discourage what I think of as "shotgun" testing - taking some existing comprehensive test for a particular feature and using it to test a mostly orthogonal feature. The orthogonal feature should have more targeted tests/it shouldn't be using such broad testing in the regression suite here.
> My take is that `clang-repl` is basically clang that takes inputs incrementally. Being that, means that we should be in a position to process whatever clang processes and thus we run against all of the existing tests. We planned to add the ones which we did not support as regression tests.
> 
> We can add more targeted tests but they would be copies or simplifications of already existing ones. Hence there is my hesitation - reuse or duplication...
> 
> My take is that clang-repl is basically clang that takes inputs incrementally. Being that, means that we should be in a position to process whatever clang processes and thus we run against all of the existing tests.

Yeah, that's sort of the "proof by absurdity" - we wouldn't want every clang test running in both ahead of time and incremental mode in the usual "check-clang" regression suite (I wouldn't mind having a separate mode for testing - more of an integration test that some buildbots or those working on more comprehensive clang-repl support could run, but most people/especially fast bots would not). So then the question for me is which tests should we have running all the time in "check-clang" - and my general answer is: Situations that have motivated code changes/support in clang-repl: If no code was added/changed/etc to clang-repl, then no test should be added to "check-clang" for that test case.

If that "run everything under check-clang run under clang-repl to find missing functionality" found some clang test that didn't work with clang-repl, yeah, I'd generally be in favour of not reusing an input or duplicating in its entirety - but reducing the test case to test only the specific clang-repl functionality issue, and testing that in particular.

Like we shouldn't test every feature of static-assert with clang-repl and clang in every "clang-check" if most of those features aren't distinctly interesting in both cases. Just enough in clang-repl to test what makes static-assert interesting in clang-repl.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125946/new/

https://reviews.llvm.org/D125946