v.g.vassilev 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 ---------------- dblaikie wrote: > 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. @dblaikie, that makes sense to me. @Purva-Chaudhari, can you address it by having just something like `// RUN: clang-repl -Xcc -E` to exercise the -E and likewise for the `EmitBC` case? 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
