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

2014-10-08 Thread Tim Ruehsen 
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-exitcode01 --leak-check=full --
track-origins=yes"
./Test-N-smaller.px

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

Tim
From df20155d1c481460fcacd578648e7be9e0eaa49f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim Rühsen?= 
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-exitcode01 --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-exitcode01 --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_ENVIRONMEN

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

2014-10-08 Thread Darshit Shah 

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?= 
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

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

2014-10-09 Thread Tim Ruehsen 
On Wednesday 08 October 2014 22:22:12 Darshit Shah wrote:
> 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.

Only if you already have been hacked. And don't forget, 'making' source code 
that you downloaded from somewhere is a *HUGE* security thread.

But if you are not hacked and trust Wget source code, in what way do you think 
is an exported variable a security thread ? Could you give me an example ?
And even if, what about variables like EDITOR which how a command (e.g. for 
crontab -e) ? It seems pretty standard to have ENV vars with commands in it.

> 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.

No, it won't. You want to valgrind Wget and not Testt-O.px, which doesn't work 
anyways since it is a script.
With my patch you are able to do that:
VALGRIND_TESTS="valgrind --tool=callgrind" ./Test-O.px

Each developer will organize it in a different way.
My personally approach would be to put 
VALGRIND_MEMCHECK="valgrind..."
VALGRIND_CALLGRIND="valgrind --tool=callgrind..."
into .bashrc

And when needed simply type
VALGRIND_TESTS=$VALGRIND_MEMCHECK ./Test-O.px

or if needed more often
export VALGRIND_TESTS=$VALGRIND_MEMCHECK
./Test-O.px
(change code)
./Test-O.px
...
./Test-O.px
(done with valgrind)
unset VALGRIND_TESTS

Such workflow seems pretty common to me.

> 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.

What about removing the configure option and
if VALGRIND_TESTS is undefined or empty or "0": normal testing
if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
else: testing with command given in VALGRIND_TESTS

The above described workflow should still work, right ?
And we could do the complete test suite with
VALGRIND_TESTS="1" make check

What do you think ?

Tim


signature.asc
Description: This is a digitally signed message part.

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

2014-10-09 Thread Tim Ruehsen 
Just one correction:

> And we could do the complete test suite with
> VALGRIND_TESTS="1" make check

This is not possible - the environment variables won't be passed to the test 
programs. That is only possible via TESTS_ENVIRONMENT, like

TESTS_ENVIRONMENT="VALGRIND_TESTS=1" make check

While running single tests work like

cd tests
VALGRIND_TESTS=1 ./Test-O.px

Tim


signature.asc
Description: This is a digitally signed message part.

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

2014-10-19 Thread Darshit Shah 

On 10/09, Tim Rühsen wrote:



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.


What about removing the configure option and
if VALGRIND_TESTS is undefined or empty or "0": normal testing
if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
else: testing with command given in VALGRIND_TESTS

The above described workflow should still work, right ?
And we could do the complete test suite with
VALGRIND_TESTS="1" make check

What do you think ?


--- end quoted text ---

I actually like this idea.
case $VALGRIND_TESTS:
   "", 0) Normal tests;;
   1) Hard coded valgrind string;;
   *) Execute the provided command;;

Could you please make the relevant changes to atleast the Perl based tests and 
to configure.ac? I'm currently traveling, but I'll fix the Python tests ASAP and 
send in a patch which will work well with the aforementioned cases.


--
Thanking You,
Darshit Shah


pgpNQKuiMcpVE.pgp
Description: PGP signature

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

2014-10-22 Thread Tim Rühsen 
Am Sonntag, 19. Oktober 2014, 17:41:30 schrieb Darshit Shah:
> On 10/09, Tim Rühsen wrote:
> >> 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.
> >
> >What about removing the configure option and
> >if VALGRIND_TESTS is undefined or empty or "0": normal testing
> >if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
> >else: testing with command given in VALGRIND_TESTS
> >
> >The above described workflow should still work, right ?
> >And we could do the complete test suite with
> >VALGRIND_TESTS="1" make check
> >
> >What do you think ?
> 
> --- end quoted text ---
> 
> I actually like this idea.
> case $VALGRIND_TESTS:
> "", 0) Normal tests;;
> 1) Hard coded valgrind string;;
> *) Execute the provided command;;
> 
> Could you please make the relevant changes to atleast the Perl based tests
> and to configure.ac? I'm currently traveling, but I'll fix the Python tests
> ASAP and send in a patch which will work well with the aforementioned
> cases.

Hi Darshit,

I care for that hopefully in the next days. I just building a new kitchen 
together with my wife. Thus I don't have much time to code/debug (right now 
she is out and fetches new building material, so I can read a few emails ;-)

Tim


signature.asc
Description: This is a digitally signed message part.

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

2014-10-25 Thread Tim Rühsen 
Am Sonntag, 19. Oktober 2014, 17:41:30 schrieb Darshit Shah:
> On 10/09, Tim Rühsen wrote:
> >> 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.
> >
> >What about removing the configure option and
> >if VALGRIND_TESTS is undefined or empty or "0": normal testing
> >if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
> >else: testing with command given in VALGRIND_TESTS
> >
> >The above described workflow should still work, right ?
> >And we could do the complete test suite with
> >VALGRIND_TESTS="1" make check
> >
> >What do you think ?
>
> --- end quoted text ---
>
> I actually like this idea.
> case $VALGRIND_TESTS:
> "", 0) Normal tests;;
> 1) Hard coded valgrind string;;
> *) Execute the provided command;;
>
> Could you please make the relevant changes to atleast the Perl based tests
> and to configure.ac? I'm currently traveling, but I'll fix the Python tests
> ASAP and send in a patch which will work well with the aforementioned
> cases.

Hi Darhsit,

here is the valgrind patch. Both Perl and Python test suites amended.

Tim
From df20155d1c481460fcacd578648e7be9e0eaa49f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim Rühsen?= 
Date: Wed, 8 Oct 2014 11:03:45 +0200
Subject: [PATCH] add ./configure valgrind support to test suites

---
 configure.ac  | 20 
 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-exitcode01 --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/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-exitcode01 --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;\
+ ex

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

2014-10-27 Thread Tim Rühsen 
Am Samstag, 25. Oktober 2014, 23:40:41 schrieb Tim Rühsen:
> Am Sonntag, 19. Oktober 2014, 17:41:30 schrieb Darshit Shah:
> > On 10/09, Tim Rühsen wrote:
> > >> 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.
> > >
> > >What about removing the configure option and
> > >if VALGRIND_TESTS is undefined or empty or "0": normal testing
> > >if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
> > >else: testing with command given in VALGRIND_TESTS
> > >
> > >The above described workflow should still work, right ?
> > >And we could do the complete test suite with
> > >VALGRIND_TESTS="1" make check
> > >
> > >What do you think ?
> >
> > --- end quoted text ---
> >
> > I actually like this idea.
> >
> > case $VALGRIND_TESTS:
> > "", 0) Normal tests;;
> > 1) Hard coded valgrind string;;
> > *) Execute the provided command;;
> >
> > Could you please make the relevant changes to atleast the Perl based tests
> > and to configure.ac? I'm currently traveling, but I'll fix the Python
> > tests
> > ASAP and send in a patch which will work well with the aforementioned
> > cases.
>
> Hi Darhsit,
>
> here is the valgrind patch. Both Perl and Python test suites amended.
>
> Tim

Added the ChangeLog entries.

Tim
From fc82fe93512e52b373800b1489efffb68b21b72e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim Rühsen?= 
Date: Wed, 8 Oct 2014 11:03:45 +0200
Subject: [PATCH 2/2] add ./configure valgrind support to test suites

---
 ChangeLog |  4 
 configure.ac  | 20 
 testenv/ChangeLog |  6 ++
 testenv/Makefile.am   |  3 ++-
 testenv/README|  6 --
 testenv/test/base_test.py | 11 +++
 tests/ChangeLog   |  5 +
 tests/Makefile.am |  3 ++-
 tests/WgetTests.pm| 17 +
 9 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 51644a1..9998c7e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2014-10-25  Tim Ruehsen 

+	* configure.ac: add --enable-valgrind-tests
+
+2014-10-25  Tim Ruehsen 
+
 	* configure.ac: check for strlcpy()

 2014-10-22  Ángel González 
diff --git a/configure.ac b/configure.ac
index 56a4767..88401cf 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_TESTS="1"
+AC_SUBST(VALGRIND_TESTS)
+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/ChangeLog b/testenv/ChangeLog
index 18087b6..c57a431 100644
--- a/testenv/ChangeLog
+++ b/testenv/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-25  Tim Ruehsen 
+
+	* test/base_test.py (gen_cmd_line): generate valgrind command line if requested
+	* README: amend description of VALGRIND_TESTS
+	* Makefile.am: set/export VALGRIND_TESTS
+
 2014-10-01  Darshit Shah  

 	* Makefile.am: Run the tests in Python's Optimizedmode
diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index b1f6781..33604bc 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -27,7 +27,8 @@


 AUTOMAKE_OPTIONS = parallel-tests
-AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK; export PYTHONPATH=$$PYTHONPATH:$(srcdir);
+AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK;\
+ export PYTHONPATH=$$PYTHONPATH:$(srcdir); export VALGRIND_TESTS="@VALGRIND_TESTS@";
 TESTS = Test-auth-basic-fail.py \
 Test-auth-basic.py  \
 Test-auth-both.py   \
diff --git a/testenv/README b/testenv/README
index 413e12e..081a957 100644
--- a/testenv/README
+++ b/testenv/README
@@ -93,8 +93,10 @@ 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 val

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

2014-10-28 Thread Tim Ruehsen 
On Monday 27 October 2014 21:59:28 Tim Rühsen wrote:
> Am Samstag, 25. Oktober 2014, 23:40:41 schrieb Tim Rühsen:
> > Am Sonntag, 19. Oktober 2014, 17:41:30 schrieb Darshit Shah:
> > > On 10/09, Tim Rühsen wrote:
> > > >> 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.
> > > >
> > > >What about removing the configure option and
> > > >if VALGRIND_TESTS is undefined or empty or "0": normal testing
> > > >if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
> > > >else: testing with command given in VALGRIND_TESTS
> > > >
> > > >The above described workflow should still work, right ?
> > > >And we could do the complete test suite with
> > > >VALGRIND_TESTS="1" make check
> > > >
> > > >What do you think ?
> > > 
> > > --- end quoted text ---
> > > 
> > > I actually like this idea.
> > > 
> > > case $VALGRIND_TESTS:
> > > "", 0) Normal tests;;
> > > 1) Hard coded valgrind string;;
> > > *) Execute the provided command;;
> > > 
> > > Could you please make the relevant changes to atleast the Perl based
> > > tests
> > > and to configure.ac? I'm currently traveling, but I'll fix the Python
> > > tests
> > > ASAP and send in a patch which will work well with the aforementioned
> > > cases.
> > 
> > Hi Darhsit,
> > 
> > here is the valgrind patch. Both Perl and Python test suites amended.
> > 
> Added the ChangeLog entries.

I pushed it.

Tim


signature.asc
Description: This is a digitally signed message part.

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

2014-10-28 Thread Pär Karlsson 
Regarding the Perl test suite, I'd be happy to clean up the code a little
bit, if you think it's worth it?

There are some minor issues with the current code (such as perl producing
certain warnings regarding invalid operators, i.e: "Argument "" isn't
numeric in numeric eq (==) at WgetTests.pm line 97."), but nothing serious
(and nothing affecting the running of the test suite per se). I'd be happy,
though, to come up with a suggestion for somewhat cleaner but functionally
equivalent code.

/Pär

2014-10-28 12:34 GMT+01:00 Tim Ruehsen :

> On Monday 27 October 2014 21:59:28 Tim Rühsen wrote:
> > Am Samstag, 25. Oktober 2014, 23:40:41 schrieb Tim Rühsen:
> > > Am Sonntag, 19. Oktober 2014, 17:41:30 schrieb Darshit Shah:
> > > > On 10/09, Tim Rühsen wrote:
> > > > >> 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.
> > > > >
> > > > >What about removing the configure option and
> > > > >if VALGRIND_TESTS is undefined or empty or "0": normal testing
> > > > >if VALGRIND_TESTS is "1": valgrind testing with hard-coded options
> > > > >else: testing with command given in VALGRIND_TESTS
> > > > >
> > > > >The above described workflow should still work, right ?
> > > > >And we could do the complete test suite with
> > > > >VALGRIND_TESTS="1" make check
> > > > >
> > > > >What do you think ?
> > > >
> > > > --- end quoted text ---
> > > >
> > > > I actually like this idea.
> > > >
> > > > case $VALGRIND_TESTS:
> > > > "", 0) Normal tests;;
> > > > 1) Hard coded valgrind string;;
> > > > *) Execute the provided command;;
> > > >
> > > > Could you please make the relevant changes to atleast the Perl based
> > > > tests
> > > > and to configure.ac? I'm currently traveling, but I'll fix the
> Python
> > > > tests
> > > > ASAP and send in a patch which will work well with the aforementioned
> > > > cases.
> > >
> > > Hi Darhsit,
> > >
> > > here is the valgrind patch. Both Perl and Python test suites amended.
> > >
> > Added the ChangeLog entries.
>
> I pushed it.
>
> Tim
>

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

2014-10-29 Thread Tim Ruehsen 
On Wednesday 29 October 2014 01:29:47 Pär Karlsson wrote:
> Regarding the Perl test suite, I'd be happy to clean up the code a little
> bit, if you think it's worth it?
> 
> There are some minor issues with the current code (such as perl producing
> certain warnings regarding invalid operators, i.e: "Argument "" isn't
> numeric in numeric eq (==) at WgetTests.pm line 97."), but nothing serious
> (and nothing affecting the running of the test suite per se). I'd be happy,
> though, to come up with a suggestion for somewhat cleaner but functionally
> equivalent code.
> 
> /Pär

Hi Pär,

well cleaner code (=better readable and maintainable) is always welcome.

Due to a lack of Perl experience I introduced the eq/== hickup in one of the 
last commits. I would appreciate it if you also fix it.

In the long term, we want to move all the Perl tests into the Python test 
suite (testenv/). So, please do not spend too much time into the Perl test 
suite. Invest time to move the tests and/or create new tests - e.g. if you can 
reproduce a bug (see Savannah Bug Tracker or the mailing list), create a test 
with it. I think, that really is a good starting point.

Tim


signature.asc
Description: This is a digitally signed message part.