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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
