Patchwork RFC: add a testsuite for libstdc++ pretty-printers

login
register
mail settings
Submitter Tom Tromey
Date Jan. 26, 2011, 9:32 p.m.
Message ID <m3pqrj1hcw.fsf@fleche.redhat.com>
Download mbox | patch
Permalink /patch/80553/
State New
Headers show

Comments

Tom Tromey - Jan. 26, 2011, 9:32 p.m.
This patch adds a minimal test suite for the libstdc++ pretty-printers.

I based it on Alexandre's gcc-gdb-test.exp code from the gcc test suite.
However, I needed somewhat different functionality, so I modified it in
a few ways.

Adding a new test is very simple: modify simple.cc and add a dg-final
marker to pretty-print the relevant objects.

Let me know what you think.

Tom

2011-01-26  Tom Tromey  <tromey@redhat.com>

	* testsuite/libstdc++-pp/simple.cc: New file.
	* testsuite/lib/gdb-test.exp: New file.
	* testsuite/libstdc++-pp/pp.exp: New file.
Jonathan Wakely - Jan. 26, 2011, 10:35 p.m.
On 26 January 2011 21:32, Tom Tromey wrote:
> This patch adds a minimal test suite for the libstdc++ pretty-printers.
>
> I based it on Alexandre's gcc-gdb-test.exp code from the gcc test suite.
> However, I needed somewhat different functionality, so I modified it in
> a few ways.
>
> Adding a new test is very simple: modify simple.cc and add a dg-final
> marker to pretty-print the relevant objects.
>
> Let me know what you think.

I'm not competent to review the code, but I'm very pleased to see a
way to test the printers.  I've been concerned about them bitrotting
if we change library internals and forget to change the printers.
This should also help us notice if we rely on features of a particular
version of python or gdb which fail with earlier versions, e.g. python
2.6 conditional expressions and gdb 7.1 lazy strings.
Benjamin Kosnik - Jan. 27, 2011, 5:19 p.m.
> I'm not competent to review the code, but I'm very pleased to see a
> way to test the printers.  I've been concerned about them bitrotting
> if we change library internals and forget to change the printers.

Yes! Love these tests, will keep everybody honest. This is also great
example code.

> This should also help us notice if we rely on features of a particular
> version of python or gdb which fail with earlier versions, e.g. python
> 2.6 conditional expressions and gdb 7.1 lazy strings.

Yep. Testresults should be very interesting to watch.

Go for it Tom.

-benjamin
Benjamin Kosnik - Jan. 27, 2011, 5:24 p.m.
> 	* testsuite/libstdc++-pp/simple.cc: New file.
> 	* testsuite/lib/gdb-test.exp: New file.
> 	* testsuite/libstdc++-pp/pp.exp: New file.

feel free to spell out prettyprinters instead of using pp, if you want
to make it very obvious what is going on in this directory.

we really strive for clarity given that this testsuite is so large

-benjamin
Tom Tromey - Jan. 27, 2011, 7:02 p.m.
Tom> * testsuite/libstdc++-pp/simple.cc: New file.
Tom> * testsuite/lib/gdb-test.exp: New file.
Tom> * testsuite/libstdc++-pp/pp.exp: New file.

Benjamin> feel free to spell out prettyprinters instead of using pp, if you want
Benjamin> to make it very obvious what is going on in this directory.

Benjamin> we really strive for clarity given that this testsuite is so large

No problem, I will do that.

Jonathan> This should also help us notice if we rely on features of a
Jonathan> particular version of python or gdb which fail with earlier
Jonathan> versions, e.g. python 2.6 conditional expressions and gdb 7.1
Jonathan> lazy strings.

Benjamin> Yep. Testresults should be very interesting to watch.

One potential problem is that gdb may change formatting details, causing
spurious failures.  I don't think that is a very big problem though.
For one thing, we can always change the dg-final annotations to use
regexps.

Tom
Tom Tromey - March 14, 2011, 8:33 p.m.
Tom> * testsuite/libstdc++-pp/simple.cc: New file.
Tom> * testsuite/lib/gdb-test.exp: New file.
Tom> * testsuite/libstdc++-pp/pp.exp: New file.

Benjamin> feel free to spell out prettyprinters instead of using pp, if you want
Benjamin> to make it very obvious what is going on in this directory.

Benjamin> we really strive for clarity given that this testsuite is so large

Tom> No problem, I will do that.

I updated my patch for this.

These new tests require a very new gdb in order to fully pass.  In
particular these tests pointed out some buglets that are only fixed in
CVS, not in any release.


Trying to dynamically adjust the tests for the gdb version looks like a
pain.

Maybe I could change the tests to check the gdb version and xfail the
tests for gdb < 7.3.  Would that be ok?

Or I could leave it as-is, but I assume that nobody wants to see new
FAILs.

Tom
Jonathan Wakely - March 14, 2011, 8:38 p.m.
On 14 March 2011 20:33, Tom Tromey wrote:
> Tom> * testsuite/libstdc++-pp/simple.cc: New file.
> Tom> * testsuite/lib/gdb-test.exp: New file.
> Tom> * testsuite/libstdc++-pp/pp.exp: New file.
>
> Benjamin> feel free to spell out prettyprinters instead of using pp, if you want
> Benjamin> to make it very obvious what is going on in this directory.
>
> Benjamin> we really strive for clarity given that this testsuite is so large
>
> Tom> No problem, I will do that.
>
> I updated my patch for this.
>
> These new tests require a very new gdb in order to fully pass.  In
> particular these tests pointed out some buglets that are only fixed in
> CVS, not in any release.
>
>
> Trying to dynamically adjust the tests for the gdb version looks like a
> pain.
>
> Maybe I could change the tests to check the gdb version and xfail the
> tests for gdb < 7.3.  Would that be ok?

That works for me, currently noone can test the printers, so if we get
to a point where people with a new enough gdb can test them that's a
huge improvement.

> Or I could leave it as-is, but I assume that nobody wants to see new
> FAILs.

Not really :-)
Benjamin Kosnik - March 14, 2011, 9:26 p.m.
> Trying to dynamically adjust the tests for the gdb version looks like
> a pain.
> 
> Maybe I could change the tests to check the gdb version and xfail the
> tests for gdb < 7.3.  Would that be ok?

Yeah.
 
> Or I could leave it as-is, but I assume that nobody wants to see new
> FAILs.

Right. Unsupported makes more sense.

-benjamin
Mike Stump - March 14, 2011, 9:27 p.m.
On Mar 14, 2011, at 1:33 PM, Tom Tromey wrote:
> These new tests require a very new gdb in order to fully pass.  In
> particular these tests pointed out some buglets that are only fixed in
> CVS, not in any release.
> 
> 
> Trying to dynamically adjust the tests for the gdb version looks like a
> pain.
> 
> Maybe I could change the tests to check the gdb version and xfail the
> tests for gdb < 7.3.  Would that be ok?

Almost sounds more like unsupported.  I'd just have a dynamic gdb check that tests a single feature and then gate all the others on it.  If you test one of the last features to go in, you can then have them all turn off for the released gdbs, and when the new gdb is released, then they all just pop on.  For hard core developers (those that have gdb trunk installed and do gcc trunk development) , they all pop on when they do an update/update install.

Also, this is better than merely checking gdb versions, as someone could add support for the printers to their version of gdb (or gdb clone), and all the testcases would pop on, once they get the first one working.

That said, the above is just a comment.

Patch

Index: testsuite/libstdc++-pp/pp.exp
===================================================================
--- testsuite/libstdc++-pp/pp.exp	(revision 0)
+++ testsuite/libstdc++-pp/pp.exp	(revision 0)
@@ -0,0 +1,46 @@ 
+#   Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with this program; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+load_lib gdb-test.exp
+
+dg-init
+v3-build_support
+
+global GDB
+if ![info exists ::env(GUALITY_GDB_NAME)] {
+    if [info exists GDB] {
+	set guality_gdb_name "$GDB"
+    } else {
+	set guality_gdb_name "[transform gdb]"
+    }
+    setenv GUALITY_GDB_NAME "$guality_gdb_name"
+}
+
+# This can be used to keep the .exe around.  dg-test has an option for
+# this but there is no way to pass it through dg-runtest.
+global dg-interpreter-batch-mode
+set dg-interpreter-batch-mode 1
+
+global DEFAULT_CXXFLAGS
+global PCH_CXXFLAGS
+dg-runtest [lsort [glob $srcdir/$subdir/*.cc]] \
+  "" "$DEFAULT_CXXFLAGS $PCH_CXXFLAGS"
+
+if [info exists guality_gdb_name] {
+    unsetenv GUALITY_GDB_NAME
+}
+
+dg-finish
Index: testsuite/libstdc++-pp/simple.cc
===================================================================
--- testsuite/libstdc++-pp/simple.cc	(revision 0)
+++ testsuite/libstdc++-pp/simple.cc	(revision 0)
@@ -0,0 +1,87 @@ 
+// { dg-do run }
+// { dg-options "-g" }
+
+// Copyright (C) 2011 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <string>
+#include <deque>
+#include <bitset>
+#include <iostream>
+#include <list>
+#include <map>
+
+template<class T>
+void
+placeholder(const T &s)
+{
+  std::cout << s;
+}
+
+template<class T, class S>
+void
+placeholder(const std::pair<T,S> &s)
+{
+  std::cout << s.first;
+}
+
+template<class T>
+void
+use(const T &container)
+{
+  for (typename T::const_iterator i = container.begin();
+       i != container.end();
+       ++i)
+    placeholder(*i);
+}
+
+int
+main()
+{
+  std::string str = "zardoz";
+// { dg-final { note-test str "\"zardoz\"" } }
+
+  std::bitset<10> bs;
+  bs[0] = 1;
+  bs[5] = 1;
+  bs[7] = 1;
+// { dg-final { note-test bs {std::bitset = {[0] = 1, [5] = 1, [7] = 1}} } }
+
+  std::deque<std::string> deq;
+  deq.push_back("one");
+  deq.push_back("two");
+// { dg-final { note-test deq {std::deque with 2 elements = {"one", "two"}} } }
+
+  std::list<std::string> lst;
+  lst.push_back("one");
+  lst.push_back("two");
+// { dg-final { note-test lst {std::list = {[0] = "one", [1] = "two"}} } }
+
+  std::map<std::string, int> mp;
+  mp["zardoz"] = 23;
+// { dg-final { note-test mp {std::map with 1 elements = {["zardoz"] = 23}} } }
+
+  placeholder(str); // Mark SPOT
+  std::cout << bs;
+  use(deq);
+  use(lst);
+  use(mp);
+
+  return 0;
+}
+
+// { dg-final { gdb-test SPOT } }
Index: testsuite/lib/gdb-test.exp
===================================================================
--- testsuite/lib/gdb-test.exp	(revision 0)
+++ testsuite/lib/gdb-test.exp	(revision 0)
@@ -0,0 +1,162 @@ 
+#   Copyright (C) 2009, 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+global gdb_tests
+set gdb_tests {}
+
+# Scan a file for markers and fill in the gdb_marker array for that
+# file.  Any error in this script is simply thrown; errors here are
+# programming errors in the test suite itself and should not be
+# caught.
+proc scan_gdb_markers {filename} {
+    global gdb_markers
+
+    if {[info exists gdb_markers($filename,-)]} {
+	return
+    }
+
+    set fd [open $filename]
+    set lineno 1
+    while {! [eof $fd]} {
+	set line [gets $fd]
+	if {[regexp -- "Mark (\[a-zA-Z0-9\]+)" $line ignore marker]} {
+	    set gdb_markers($filename,$marker) $lineno
+	}
+	incr lineno
+    }
+    close $fd
+
+    set gdb_markers($filename,-) {}
+}
+
+# Find a marker in a source file, and return the marker's line number.
+proc get_line_number {filename marker} {
+    global gdb_markers
+
+    scan_gdb_markers $filename
+    return $gdb_markers($filename,$marker)
+}
+
+# Make note of a gdb test.  A test consists of a variable name and an
+# expected result.
+proc note-test {var result} {
+    global gdb_tests
+
+    lappend gdb_tests $var $result
+}
+
+# Utility for testing variable values using gdb, invoked via dg-final.
+# Tests all tests indicated by note-test.
+#
+# Argument 0 is the marker on which to put a breakpoint
+# Argument 2 handles expected failures and the like
+proc gdb-test { marker {selector {}} } {
+    if { ![isnative] || [is_remote target] } { return }
+
+    if {[string length $selector] > 0} {
+	switch [dg-process-target $selector] {
+	    "S" { }
+	    "N" { return }
+	    "F" { setup_xfail "*-*-*" }
+	    "P" { }
+	}
+    }
+
+    # This assumes that we are three frames down from dg-test, and that
+    # it still stores the filename of the testcase in a local variable "name".
+    # A cleaner solution would require a new DejaGnu release.
+    upvar 2 name testcase
+    upvar 2 prog prog
+
+    set line [get_line_number $prog $marker]
+
+    set gdb_name $::env(GUALITY_GDB_NAME)
+    set testname "$testcase"
+    set output_file "[file rootname [file tail $prog]].exe"
+    set cmd_file "[file rootname [file tail $prog]].gdb"
+
+    global srcdir
+    set pycode [file join $srcdir .. python libstdcxx v6 printers.py]
+
+    global gdb_tests
+
+    set fd [open $cmd_file "w"]
+    puts $fd "source $pycode"
+    puts $fd "python register_libstdcxx_printers(None)"
+    puts $fd "break $line"
+    puts $fd "run"
+
+    set count 0
+    foreach {var result} $gdb_tests {
+	puts $fd "print $var"
+	incr count
+	set gdb_var($count) $var
+	set gdb_expected($count) $result
+    }
+    set gdb_tests {}
+
+    puts $fd "quit"
+    close $fd
+
+    send_log "Spawning: $gdb_name -nx -nw -quiet -batch -x $cmd_file ./$output_file\n"
+    set res [remote_spawn target "$gdb_name -nx -nw -quiet -batch -x $cmd_file ./$output_file"]
+    if { $res < 0 || $res == "" } {
+	unsupported "$testname"
+	return
+    }
+
+    remote_expect target [timeout_value] {
+	-re {^\$([0-9]+) = ([^\n\r]*)[\n\r]+} {
+	    set num $expect_out(1,string)
+	    set first $expect_out(2,string)
+	    if { ![string compare $first $gdb_expected($num)] } {
+		pass "$testname print $gdb_var($num)"
+	    } else {
+		fail "$testname print $gdb_var($num)"
+		verbose "     got =>$first<="
+		verbose "expected =>$gdb_expected($num)<="
+	    }
+
+	    if {$num == $count} {
+		remote_close target
+		return
+	    } else {
+		exp_continue
+	    }
+	}
+
+	-re {Python scripting is not supported in this copy of GDB.[\n\r]+} {
+	    unsupported "$testname"
+	    remote_close target
+	    return
+	}
+
+	-re {^[^$][^\n\r]*[\n\r]+} {
+	    verbose "skipping: $expect_out(buffer)"
+	    exp_continue
+	}
+
+	timeout {
+	    unsupported "$testname"
+	    remote_close target
+	    return
+	}
+    }
+
+    remote_close target
+    unsupported "$testname"
+    return
+}