Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

Started by Michael Paquieralmost 5 years ago7 messages

michael@paquier.xyz

almost 5 years ago

1 attachment(s)

Hi all,

Each time I do development on Windows, I get annoyed by the fact that
it is not easy to run individual test scripts in the same way as we do
on any other platforms, using PROVE_TESTS, or even PROVE_FLAGS. And
there have been recent complaints about not being able to do that.

Please find attached a patch to support both variables, with some
documentation. Using a terminal on Windows, one can set those
variables using just.. "set", say:
set PROVE_FLAGS=--timer
set PROVE_TESTS=t/020*.pl t/010*.pl

Note the absence of quotes for the second one, so as it is possible to
apply cleanly glob() to each element passed down.

Thoughts?
--
Michael

Attachments:

win32-tap-flags.patchtext/x-diff; charset=us-asciiDownload

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 47e5f7c8ae..3692b49b39 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -496,6 +496,19 @@ $ENV{PERL5LIB}=$ENV{PERL5LIB} . ';c:\IPC-Run-0.94\lib';
     </varlistentry>
    </variablelist>
   </para>
+
+  <para>
+   The TAP tests run with <command>vcregress</command> support the
+   environment variables <varname>PROVE_TESTS</varname>, that is expanded
+   automatically using the name patterns given, and
+   <varname>PROVE_FLAGS</varname>. These can for instance be set
+   on a Windows terminal as follows, before running
+   <command>vcregress</command>:
+<programlisting>
+set PROVE_FLAGS=--timer
+set PROVE_TESTS=t/020*.pl t/010*.pl
+</programlisting>
+  </para>
  </sect2>
 
  </sect1>
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 266098e193..d6c33f68ee 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -209,7 +209,21 @@ sub tap_check
 	my $dir = shift;
 	chdir $dir;
 
-	my @args = ("prove", @flags, glob("t/*.pl"));
+	# Fetch and adjust PROVE_TESTS, applying glob() to each element
+	# defined to get and list individual tests.
+	my $prove_tests_val = $ENV{PROVE_TESTS} || "t/*.pl";
+	my @prove_tests_array = split(/\s+/, $prove_tests_val);
+	my @prove_tests = ();
+	foreach (@prove_tests_array)
+	{
+		push(@prove_tests, glob($_));
+	}
+
+	# Fetch and adjust PROVE_FLAGS, handling multiple arguments.
+	my $prove_flags_val = $ENV{PROVE_FLAGS} || "";
+	my @prove_flags = split(/\s+/, $prove_flags_val);
+
+	my @args = ("prove", @flags, @prove_tests, @prove_flags);
 
 	# adjust the environment for just this test
 	local %ENV = %ENV;

Julien Rouhaud

rjuju123@gmail.com

almost 5 years ago

In reply to: Michael Paquier (#1)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Wed, Mar 03, 2021 at 05:19:22PM +0900, Michael Paquier wrote:

Each time I do development on Windows, I get annoyed by the fact that
it is not easy to run individual test scripts in the same way as we do
on any other platforms, using PROVE_TESTS, or even PROVE_FLAGS. And
there have been recent complaints about not being able to do that.

Please find attached a patch to support both variables, with some
documentation. Using a terminal on Windows, one can set those
variables using just.. "set", say:
set PROVE_FLAGS=--timer
set PROVE_TESTS=t/020*.pl t/010*.pl

Note the absence of quotes for the second one, so as it is possible to
apply cleanly glob() to each element passed down.

Thoughts?

+1, I heavily rely on that and I can imagine how hard it's to develop a patch
on windows without it.

Patch LGTM, althouth I don't have any Windows environnment to test it.

Juan José Santamaría Flecha

juanjo.santamaria@gmail.com

almost 5 years ago

In reply to: Julien Rouhaud (#2)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Wed, Mar 3, 2021 at 11:25 AM Julien Rouhaud <rjuju123@gmail.com> wrote:

On Wed, Mar 03, 2021 at 05:19:22PM +0900, Michael Paquier wrote:

Each time I do development on Windows, I get annoyed by the fact that
it is not easy to run individual test scripts in the same way as we do
on any other platforms, using PROVE_TESTS, or even PROVE_FLAGS. And
there have been recent complaints about not being able to do that.

Please find attached a patch to support both variables, with some
documentation. Using a terminal on Windows, one can set those
variables using just.. "set", say:
set PROVE_FLAGS=--timer
set PROVE_TESTS=t/020*.pl t/010*.pl

Note the absence of quotes for the second one, so as it is possible to
apply cleanly glob() to each element passed down.

Thoughts?

+1, I heavily rely on that and I can imagine how hard it's to develop a
patch
on windows without it.

Patch LGTM, althouth I don't have any Windows environnment to test it.

+1 for the functionality. I have tested it and works as expected in my
environment (Windows 10 + VS 2019).

Just a comment about the documentation:

+  <para>
+   The TAP tests run with <command>vcregress</command> support the
+   environment variables <varname>PROVE_TESTS</varname>, that is expanded
+   automatically using the name patterns given, and
+   <varname>PROVE_FLAGS</varname>. These can for instance be set
+   on a Windows terminal as follows, before running
+   <command>vcregress</command>:
+<programlisting>
+set PROVE_FLAGS=--timer
+set PROVE_TESTS=t/020*.pl t/010*.pl
+</programlisting>
+  </para>

This seems to me as if setting the variables in the shell is the proposed
way to do so. In the previous doc point we do the same with the buildenv.pl
file. It looks inconsistent, as if it was one way or the other, when it
could be either.

Regards,

Juan José Santamaría Flecha

Michael Paquier

michael@paquier.xyz

almost 5 years ago

In reply to: Juan José Santamaría Flecha (#3)

1 attachment(s)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Wed, Mar 03, 2021 at 08:59:30PM +0100, Juan José Santamaría Flecha wrote:

This seems to me as if setting the variables in the shell is the proposed
way to do so. In the previous doc point we do the same with the buildenv.pl
file. It looks inconsistent, as if it was one way or the other, when it
could be either.

Okay, that makes sense. PROVE_TESTS is a runtime-only dependency so
my guess is that most people would set that directly on the command
prompt like I do, still it can be useful to enforce that in build.pl.
PROVE_FLAGS can be both, but you are right that setting it in build.pl
would be the most common approach. So let's document both. Attached
is a proposal for that, what do you think? I have extended the
example of PROVE_FLAGS to show how to set up more --jobs.
--
Michael

Attachments:

win32-tap-flags-v2.patchtext/x-diff; charset=us-asciiDownload

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 47e5f7c8ae..b84d8ee2b1 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -496,6 +496,24 @@ $ENV{PERL5LIB}=$ENV{PERL5LIB} . ';c:\IPC-Run-0.94\lib';
     </varlistentry>
    </variablelist>
   </para>
+
+  <para>
+   The TAP tests run with <command>vcregress</command> support the
+   environment variables <varname>PROVE_TESTS</varname>, that is expanded
+   automatically using the name patterns given, and
+   <varname>PROVE_FLAGS</varname>. These can be set on a Windows terminal,
+   before running <command>vcregress</command>:
+<programlisting>
+set PROVE_FLAGS=--timer --jobs 2
+set PROVE_TESTS=t/020*.pl t/010*.pl
+</programlisting>
+   It is also possible to set up those parameters in
+   <filename>buildenv.pl</filename>:
+<programlisting>
+$ENV{PROVE_FLAGS}=$ENV{PROVE_FLAGS} . '--timer --jobs 2'
+$ENV{PROVE_TESTS}=$ENV{PROVE_TESTS} . 't/020*.pl t/010*.pl'
+</programlisting>
+  </para>
  </sect2>
 
  </sect1>
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 266098e193..d6c33f68ee 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -209,7 +209,21 @@ sub tap_check
 	my $dir = shift;
 	chdir $dir;
 
-	my @args = ("prove", @flags, glob("t/*.pl"));
+	# Fetch and adjust PROVE_TESTS, applying glob() to each element
+	# defined to get and list individual tests.
+	my $prove_tests_val = $ENV{PROVE_TESTS} || "t/*.pl";
+	my @prove_tests_array = split(/\s+/, $prove_tests_val);
+	my @prove_tests = ();
+	foreach (@prove_tests_array)
+	{
+		push(@prove_tests, glob($_));
+	}
+
+	# Fetch and adjust PROVE_FLAGS, handling multiple arguments.
+	my $prove_flags_val = $ENV{PROVE_FLAGS} || "";
+	my @prove_flags = split(/\s+/, $prove_flags_val);
+
+	my @args = ("prove", @flags, @prove_tests, @prove_flags);
 
 	# adjust the environment for just this test
 	local %ENV = %ENV;

Juan José Santamaría Flecha

juanjo.santamaria@gmail.com

almost 5 years ago

In reply to: Michael Paquier (#4)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Thu, Mar 4, 2021 at 3:11 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Mar 03, 2021 at 08:59:30PM +0100, Juan José Santamaría Flecha
wrote:

This seems to me as if setting the variables in the shell is the proposed
way to do so. In the previous doc point we do the same with the

buildenv.pl

file. It looks inconsistent, as if it was one way or the other, when it
could be either.

Okay, that makes sense. PROVE_TESTS is a runtime-only dependency so
my guess is that most people would set that directly on the command
prompt like I do, still it can be useful to enforce that in build.pl.
PROVE_FLAGS can be both, but you are right that setting it in build.pl
would be the most common approach. So let's document both. Attached
is a proposal for that, what do you think? I have extended the
example of PROVE_FLAGS to show how to set up more --jobs.

LGTM.

Regards,

Juan José Santamaría Flecha

Michael Paquier

michael@paquier.xyz

almost 5 years ago

In reply to: Juan José Santamaría Flecha (#5)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Thu, Mar 04, 2021 at 06:37:56PM +0100, Juan José Santamaría Flecha wrote:

LGTM.

Thanks. I have tested more combinations of options, came back a bit
to the documentation for buildenv.pl where copying the values from the
docs would result in a warning, and applied it.
--
Michael

Julien Rouhaud

rjuju123@gmail.com

almost 5 years ago

In reply to: Michael Paquier (#6)

Re: Add support for PROVE_FLAGS and PROVE_TESTS in MSVC scripts

On Fri, Mar 5, 2021 at 9:28 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Mar 04, 2021 at 06:37:56PM +0100, Juan José Santamaría Flecha wrote:

LGTM.

Thanks. I have tested more combinations of options, came back a bit
to the documentation for buildenv.pl where copying the values from the
docs would result in a warning, and applied it.

Great news!