bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] [PATCH] Add valgrind testing support via ./configure


From: Darshit Shah
Subject: Re: [Bug-wget] [PATCH] Add valgrind testing support via ./configure
Date: Wed, 8 Oct 2014 22:22:12 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

On 10/08, Tim Rühsen wrote:
Hi,

this patch allows for using valgrind memcheck for both test suites via

        ./configure --enable-valgrind-tests

With valgrind memcheck 24 (perl) tests fail due to lost memory blocks.
The python tests all survive (thanks to Darshits thorough testing).

Since valgrind is pretty slow, you can of course start a single test with
valgrind like

        cd tests
        export VALGRIND_TESTS="valgrind --error-exitcode=301 --leak-check=full 
--
track-origins=yes"
        ./Test-N-smaller.px

This allows also testing with other tools or valgrind options (e.g. valgrind
--tool=callgrind for performance tuning).

Tim

Hi Tim,

I'm not entirely sold on the idea of passing a complete command line and executing it through the tests. At the very least its a minor security vulnerability for everyone running make check. An exported environment variable could then lead to arbitrary code execution.

I cannot generalize this next point, but from my own personal use case, valgrind based tests are run when making some changes to the source which could potentially lead to memory leaks. In such a case, I would want to run a single test under valgrind, and not the complete test suite. Having to type the complete valgrind command before executing the test, or exporting a relevant environment variable earlier seems like a bad way to do it. I would prefer that the command be hard coded into the test suites itself. If a more specialized command needs to be run, the user can always run it directly:
valgrind --tool=callgrind ./Test-O.px
Will still work.

Hence, hard coding the command actually reduces the amount of work a user needs to do in order to run the tests under valgrind.

My suggestion is that we allow the configure option, but hard code the valgrind command into the test suites themselves, and not leave them environment variables.

From df20155d1c481460fcacd578648e7be9e0eaa49f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20R=C3=BChsen?= <address@hidden>
Date: Wed, 8 Oct 2014 11:03:45 +0200
Subject: [PATCH] add ./configure valgrind support to test suites

---
configure.ac              | 20 ++++++++++++++++++++
testenv/Makefile.am       |  3 ++-
testenv/README            |  5 +++--
testenv/test/base_test.py |  6 +-----
tests/Makefile.am         |  3 ++-
tests/WgetTests.pm        | 11 +++++++----
6 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3cbe618..19a72df 100644
--- a/configure.ac
+++ b/configure.ac
@@ -101,6 +101,25 @@ test x"${ENABLE_DEBUG}" = xyes && 
AC_DEFINE([ENABLE_DEBUG], 1,
   [Define if you want the debug output support compiled in.])

dnl
+dnl Check for valgrind
+dnl
+AC_ARG_ENABLE(valgrind-tests,
+  AS_HELP_STRING([--enable-valgrind-tests], [enable using Valgrind for tests]),
+  [ac_enable_valgrind=$enableval], [ac_enable_valgrind=no])
+if test "${ac_enable_valgrind}" != "no" ; then
+  AC_CHECK_PROG(HAVE_VALGRIND, valgrind, yes, no)
+  if test "$HAVE_VALGRIND" = "yes" ; then
+    VALGRIND_ENVIRONMENT="valgrind --error-exitcode=301 --leak-check=full 
--track-origins=yes"
+    AC_SUBST(VALGRIND_ENVIRONMENT)
+    VALGRIND_INFO="Test suite will be run under Valgrind"
+  else
+    VALGRIND_INFO="Valgrind not found"
+  fi
+else
+  VALGRIND_INFO="Valgrind testing not enabled"
+fi
+
+dnl
dnl Find the compiler
dnl

@@ -599,4 +618,5 @@ AC_MSG_NOTICE([Summary of build options:
  NTLM:              $ENABLE_NTLM
  OPIE:              $ENABLE_OPIE
  Debugging:         $ENABLE_DEBUG
+  Valgrind:          $VALGRIND_INFO
])
diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index a83267f..ff423c2 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -56,6 +56,7 @@ TESTS = $(PY_TESTS)
XFAIL_TESTS = $(PY_XFAIL_TESTS)

TEST_EXTENSIONS = .py
-AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; export 
SYSTEM_WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK; export 
PYTHONPATH=$$PYTHONPATH:$(srcdir);
+AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; export 
SYSTEM_WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK;\
+ export PYTHONPATH=$$PYTHONPATH:$(srcdir); export 
VALGRIND_TESTS="@VALGRIND_ENVIRONMENT@";
PY_LOG_COMPILER = python3
#AM_PY_LOG_FLAGS = diff --git a/testenv/README b/testenv/README
index 413e12e..a498452 100644
--- a/testenv/README
+++ b/testenv/README
@@ -93,8 +93,9 @@ Environment Variables:
  valgrind.
* NO_CLEANUP: Do not remove the temporary files created by the test.
  This will prevent the ${testname}-test directory from being deleted
-* VALGRIND_TESTS: If this variable is set, the test suite will execute all the
-  tests through valgrind's memcheck tool.
+* VALGRIND_TESTS: If this variable is set and contains the valgrind command 
line,
+  the test suite will execute all the tests via this command.
+  This variable is set to valgrind memcheck by ./configure 
--enable-valgrind-tests.


File Structure:
diff --git a/testenv/test/base_test.py b/testenv/test/base_test.py
index 14143c2..e058f82 100644
--- a/testenv/test/base_test.py
+++ b/testenv/test/base_test.py
@@ -100,11 +100,7 @@ class BaseTest:
                                                 "..", '..', 'src', "wget"))
        wget_options = '--debug --no-config %s' % self.wget_options

-        if os.getenv("VALGRIND_TESTS"):
-            valgrind_test = "valgrind --error-exitcode=301 --leak-check=full 
--track-origins=yes"
-        else:
-            valgrind_test = ""
-        cmd_line = '%s %s %s ' % (valgrind_test, wget_path, wget_options)
+        cmd_line = '%s %s %s ' % (os.getenv("VALGRIND_TESTS", ""), wget_path, 
wget_options)
        for protocol, urls, domain in zip(self.protocols,
                                          self.urls,
                                          self.domains):
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1248036..9c6c01e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -141,6 +141,7 @@ CLEANFILES = *~ *.bak core core.[0-9]*

TESTS = ./unit-tests$(EXEEXT) $(PX_TESTS)
TEST_EXTENSIONS = .px
-AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; export SYSTEM_WGETRC=/dev/null;
PX_LOG_COMPILER = $(PERL)
AM_PX_LOG_FLAGS = -I$(srcdir)
+AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; export 
SYSTEM_WGETRC=/dev/null;\
+ export VALGRIND_TESTS="@VALGRIND_ENVIRONMENT@";
diff --git a/tests/WgetTests.pm b/tests/WgetTests.pm
index 1c65d54..fe0f722 100644
--- a/tests/WgetTests.pm
+++ b/tests/WgetTests.pm
@@ -88,13 +88,16 @@ sub run {

    # Call wget
    chdir ("$self->{_workdir}/$self->{_name}/output");
+
    my $cmdline = $self->{_cmdline};
    $cmdline = $self->_substitute_port($cmdline);
+    $cmdline = ($cmdline =~ m{^/.*}) ? $cmdline : "$self->{_workdir}/$cmdline";
+
+    my $valgrind = $ENV{VALGRIND_TESTS};
+    $cmdline = ($valgrind ? $valgrind . " " : "") . $cmdline;
+
    print "Calling $cmdline\n";
-    $errcode =
-        ($cmdline =~ m{^/.*})
-            ? system ($cmdline)
-            : system ("$self->{_workdir}/$cmdline");
+    $errcode = system($cmdline);
    $errcode >>= 8; # XXX: should handle abnormal error codes.

    # Shutdown server
--
2.1.1




--- end quoted text ---

--
Thanking You,
Darshit Shah

Attachment: pgpjbAVSh7qOO.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]