On Tue, Oct 2, 2012 at 8:49 AM, Dmitri Gribenko <[email protected]> wrote:
> On Tue, Oct 2, 2012 at 6:46 PM, Alexander Kornienko <[email protected]> > wrote: > > On Tue, Oct 2, 2012 at 5:36 PM, Dmitri Gribenko <[email protected]> > wrote: > >> > >> On Tue, Oct 2, 2012 at 6:28 PM, Alexander Kornienko <[email protected]> > >> wrote: > >> > On Tue, Oct 2, 2012 at 5:15 PM, Dmitri Gribenko <[email protected]> > >> > wrote: > >> >> > >> >> On Tue, Oct 2, 2012 at 5:59 PM, Alexander Kornienko < > [email protected]> > >> >> wrote: > >> >> > So I'm definitely for using FileCheck in this case, but it's only > me, > >> >> > others > >> >> > may disagree. > >> >> > >> >> I'm in favor of ASTMatchers-based tests for anything dumping related, > >> >> based on my experience of maintaining > >> >> test/Index/annotate-comments.cpp. Any small change to the testcases > >> >> forces a change of line numbers for everything that follows it (half > >> >> of tests, on average). > >> > > >> > FileCheck has variables now, maybe we can add a special __LINE__ > >> > variable > >> > (possibly with a way to specify offset)? In this case we could put > CHECK > >> > lines near the actual test code and this would not be bound to > absolute > >> > line > >> > numbers. What do you think? > >> > >> That would help somewhat. But nevertheless, annotate-comments.cpp is > >> the slowest testcase in the testsuite (for some reason, FileCheck > >> takes up to 10s to match it in debug version -- spending most of the > >> time in regex matching). > > > > > > I'll look into adding special variables to FileCheck. > > > > As for debug build, can we somehow use release FileCheck even when > testing a > > debug configuration? > > > >> > >> >> With ASTMatchers-based tests we can keep all > >> >> logically related tests in a single file and keep the test case close > >> >> to the expected output while ensuring that different tests are > >> >> isolated from each other. > >> > > >> > Isolation is possible now using separate RUN lines with specific -D > >> > defines > >> > and #ifdef blocks. > >> > >> That's more like putting the isolation into a tool that didn't have > >> it, versus having an approach where testcases are intrinsically > >> isolated. > > > > My problem with ASTMatchers-based tests for dumping is that there are > lots > > of dependencies, that can be avoided. And the test infrastructure (a > custom > > test infrastructure, used only for one test) is an additional possible > point > > of failure, and definitely requires some support. In my opinion it makes > > sense only in case this infrastructure is generalized, moved elsewhere > and > > gets its own tests. But still this seems to be overkill. > > The infrastructure is rather simple in my opinion, and can (should!) > be shared between (Decl|Stmt)(Printer|Dumper) tests. So, generally I do agree. However, FileCheck was *designed* for testing textual output. It seems a bit weird to not be using it here... I wonder if it would make sense to hoist the FileCheck logic into a library such that the unittests could actually use the same syntax for making textual assertions from within unittests? This has been my biggest frustration with unittests checking textual output -- often the errors do not make it clear what broke or how to fix. They also tend to devolve to golden-output tests rather than using minimal assertions to check for the important output.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
