Skip Menu |
Report information
Id: 132139
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: jkeenan [at] pobox.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



To: perlbug [...] perl.org
Date: Wed, 20 Sep 2017 19:33:21 -0400
From: James E Keenan <jkeenan [...] pobox.com>
Subject: 'make minitest' non-zero error code ignored
Download (untitled) / with headers
text/plain 2.9k
As documented in https://rt.perl.org/Ticket/Display.html?id=132138, 't/run/switches.t' fails when run under 'miniperl', probably because it 'require's module File::Spec. I stumbled upon this by accident today. Following up on a discussion on IRC #p5p and in https://rt.perl.org/Ticket/Display.html?id=132092, when testing the perl-5.27.4 tarball provided by John SJ Anderson on FreeBSD-11.0, instead of saying, as I normally would: ##### $ regular_configure && make test_harness ##### ... (where 'regular_configure' is a shell script that, on FreeBSD, builds a threaded perl very close to the 'system perl'), I said: ##### $ regular_configure && make minitest && make test_harness ##### I guessed that if 'make minitest' were to FAIL, then 'make test_harness' would never be run and my attention would immediately be drawn to failing tests. I sat watching the terminal and thought I saw a test failure during 'make minitest'. But it quickly became apparent that this failure -- subsequently confirmed as being in 't/run/switches.t' did not prevent 'make test_harness' from starting to run. So I then cleaned the directory, re-configured and then said 'make minitest'. 'make minitest' did find and report the error in 't/run/switches.t' but its error code is ignored! (See gzipped attachment for complete output as reproduced on Linux.) ##### ... t/run/switchd .................. ok t/run/switches ................. Can't locate File/Spec.pm in @INC (you may need to install the File::Spec module) (@INC contains: ../lib) at run/switches.t line 424. # Looks like you planned 137 tests but ran 111. FAILED--expected 137 tests, saw 111 t/run/switchn .................. ok ... t/perf/taint ................... skipped Failed 1 test out of 343, 99.71% okay. run/switches.t ### Since not all tests were successful, you may want to run some of ### them individually and examine any diagnostic messages they produce. ### See the INSTALL document's section on "make test". ### You have a good chance to get more information by running ### ./perl harness ### in the 't' directory since most (>=80%) of the tests succeeded. ### You may have to set your dynamic library search path, ### LD_LIBRARY_PATH, to point to the build directory: ### setenv LD_LIBRARY_PATH `pwd`; cd t; ./perl harness ### LD_LIBRARY_PATH=`pwd`; export LD_LIBRARY_PATH; cd t; ./perl harness ### export LD_LIBRARY_PATH=`pwd`; cd t; ./perl harness ### for csh-style shells, like tcsh; or for traditional/modern ### Bourne-style shells, like bash, ksh, and zsh, respectively. Elapsed: 124 sec u=1.69 s=0.24 cu=39.93 cs=1.73 scripts=343 tests=348120 makefile:824: recipe for target 'minitest' failed make: [minitest] Error 1 (ignored) ##### Ignoring this error code, IMO, diminishes the value of 'make minitest' to an author or committer trying to evaluate the correctness of a patch. Is there a rationale for ignoring this error code? If not, what is the recommended fix? Thank you very much. Jim Keenan

Message body not shown because it is not plain text.

Download perl_V.txt
text/plain 3k

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.3k
On Wed, 20 Sep 2017 16:33:32 -0700, jkeenan@pobox.com wrote: Show quoted text
> As documented in https://rt.perl.org/Ticket/Display.html?id=132138, > 't/run/switches.t' fails when run under 'miniperl', probably because it > 'require's module File::Spec. > > I stumbled upon this by accident today. Following up on a discussion on > IRC #p5p and in https://rt.perl.org/Ticket/Display.html?id=132092, when > testing the perl-5.27.4 tarball provided by John SJ Anderson on > FreeBSD-11.0, instead of saying, as I normally would: > > ##### > $ regular_configure && make test_harness > ##### > > ... (where 'regular_configure' is a shell script that, on FreeBSD, > builds a threaded perl very close to the 'system perl'), I said: > > ##### > $ regular_configure && make minitest && make test_harness > ##### > > I guessed that if 'make minitest' were to FAIL, then 'make test_harness' > would never be run and my attention would immediately be drawn to > failing tests. > > I sat watching the terminal and thought I saw a test failure during > 'make minitest'. But it quickly became apparent that this failure -- > subsequently confirmed as being in 't/run/switches.t' did not prevent > 'make test_harness' from starting to run. > > So I then cleaned the directory, re-configured and then said 'make > minitest'. 'make minitest' did find and report the error in > 't/run/switches.t' but its error code is ignored! (See gzipped > attachment for complete output as reproduced on Linux.)
minitest has ignored errors since it was introduced in 16d20bd98cd29be76029ebf04027a7edd34d817b: commit 16d20bd98cd29be76029ebf04027a7edd34d817b Author: Andy Dougherty <doughera@lafcol.lafayette.edu> Date: Tue May 30 22:59:41 1995 +0000 This is my patch patch.1i for perl5.001. ... Makefile.SH ... Add 'minitest' target. ... diff --git a/Makefile.SH b/Makefile.SH index f5dff60..cdd6333 100644 --- a/Makefile.SH +++ b/Makefile.SH ... @@ -374,6 +395,11 @@ test: miniperl perl preplibrary $(dynamic_ext) - cd t && chmod +x TEST */*.t - cd t && (rm -f perl; $(LNS) ../perl perl) && ./perl TEST </dev/tty +minitest: miniperl + - cd t && chmod +x TEST */*.t + - cd t && (rm -f perl; $(LNS) ../miniperl perl) \ + && ./perl TEST base/*.t comp/*.t cmd/*.t io/*.t op/*.t </dev/tty + clist: $(c) echo $(c) | tr ' ' '\012' >.clist Tony
RT-Send-CC: perl5-porters [...] perl.org
On Thu, 21 Sep 2017 00:47:45 GMT, tonyc wrote: Show quoted text
> On Wed, 20 Sep 2017 16:33:32 -0700, jkeenan@pobox.com wrote:
> > As documented in https://rt.perl.org/Ticket/Display.html?id=132138, > > 't/run/switches.t' fails when run under 'miniperl', probably because > > it > > 'require's module File::Spec. > > > > I stumbled upon this by accident today. Following up on a discussion > > on > > IRC #p5p and in https://rt.perl.org/Ticket/Display.html?id=132092, > > when > > testing the perl-5.27.4 tarball provided by John SJ Anderson on > > FreeBSD-11.0, instead of saying, as I normally would: > > > > ##### > > $ regular_configure && make test_harness > > ##### > > > > ... (where 'regular_configure' is a shell script that, on FreeBSD, > > builds a threaded perl very close to the 'system perl'), I said: > > > > ##### > > $ regular_configure && make minitest && make test_harness > > ##### > > > > I guessed that if 'make minitest' were to FAIL, then 'make > > test_harness' > > would never be run and my attention would immediately be drawn to > > failing tests. > > > > I sat watching the terminal and thought I saw a test failure during > > 'make minitest'. But it quickly became apparent that this failure -- > > subsequently confirmed as being in 't/run/switches.t' did not prevent > > 'make test_harness' from starting to run. > > > > So I then cleaned the directory, re-configured and then said 'make > > minitest'. 'make minitest' did find and report the error in > > 't/run/switches.t' but its error code is ignored! (See gzipped > > attachment for complete output as reproduced on Linux.)
> > minitest has ignored errors since it was introduced in > 16d20bd98cd29be76029ebf04027a7edd34d817b: > > commit 16d20bd98cd29be76029ebf04027a7edd34d817b > Author: Andy Dougherty <doughera@lafcol.lafayette.edu> > Date: Tue May 30 22:59:41 1995 +0000 > > This is my patch patch.1i for perl5.001. > > ... > Makefile.SH > ... > Add 'minitest' target. > > ... > diff --git a/Makefile.SH b/Makefile.SH > index f5dff60..cdd6333 100644 > --- a/Makefile.SH > +++ b/Makefile.SH > ... > @@ -374,6 +395,11 @@ test: miniperl perl preplibrary $(dynamic_ext) > - cd t && chmod +x TEST */*.t > - cd t && (rm -f perl; $(LNS) ../perl perl) && ./perl TEST > </dev/tty > > +minitest: miniperl > + - cd t && chmod +x TEST */*.t > + - cd t && (rm -f perl; $(LNS) ../miniperl perl) \ > + && ./perl TEST base/*.t comp/*.t cmd/*.t io/*.t op/*.t > </dev/tty > + > clist: $(c) > echo $(c) | tr ' ' '\012' >.clist > > > Tony
So, on the one hand, I would argue, it does not DWIM for the purpose for which it is recommended in pod/perlgit.pod and by commenters on list today. On the other hand, to change that situation would require changing the way it has worked for 22 years (and I'm always inclined to trust Andy D's judgment). I'm inclined to the first hand. I want to use it in a string of commands and I want that string to fail if and when 'make minitest' fails. Otherwise I'm inclined to never use it. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
To: James E Keenan via RT <perlbug-followup [...] perl.org>
Date: Wed, 20 Sep 2017 22:25:06 -0400
Subject: Re: [perl #132139] 'make minitest' non-zero error code ignored
From: Andy Dougherty <doughera [...] lafayette.edu>
Download (untitled) / with headers
text/plain 1.9k
On Wed, Sep 20, 2017 at 06:54:20PM -0700, James E Keenan via RT wrote: Show quoted text
> On Thu, 21 Sep 2017 00:47:45 GMT, tonyc wrote:
Show quoted text
> > minitest has ignored errors since it was introduced in > > 16d20bd98cd29be76029ebf04027a7edd34d817b: > > > > commit 16d20bd98cd29be76029ebf04027a7edd34d817b > > Author: Andy Dougherty <doughera@lafcol.lafayette.edu> > > Date: Tue May 30 22:59:41 1995 +0000 > > > > This is my patch patch.1i for perl5.001. > > > > ... > > Makefile.SH > > ... > > Add 'minitest' target. > > > > ... > > diff --git a/Makefile.SH b/Makefile.SH > > index f5dff60..cdd6333 100644 > > --- a/Makefile.SH > > +++ b/Makefile.SH > > ... > > @@ -374,6 +395,11 @@ test: miniperl perl preplibrary $(dynamic_ext) > > - cd t && chmod +x TEST */*.t > > - cd t && (rm -f perl; $(LNS) ../perl perl) && ./perl TEST > > </dev/tty > > > > +minitest: miniperl > > + - cd t && chmod +x TEST */*.t > > + - cd t && (rm -f perl; $(LNS) ../miniperl perl) \ > > + && ./perl TEST base/*.t comp/*.t cmd/*.t io/*.t op/*.t > > </dev/tty > > + > > clist: $(c) > > echo $(c) | tr ' ' '\012' >.clist > > > > > > Tony
> > So, on the one hand, I would argue, it does not DWIM for the purpose for which it is recommended in pod/perlgit.pod and by commenters on list today. > > On the other hand, to change that situation would require changing the way it has worked for 22 years (and I'm always inclined to trust Andy D's judgment). > > I'm inclined to the first hand. I want to use it in a string of commands and I want that string to fail if and when 'make minitest' fails. Otherwise I'm inclined to never use it.
While I appreciate the thought, I don't recall exercising any judgment there. The 'minitest' target was pretty much a cut & paste of the 'test' target, which ignored the exit status. I don't recall why. Your inclination sounds sensible to me. -- Andy Dougherty doughera@lafayette.edu
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.3k
On Thu, 21 Sep 2017 02:25:47 GMT, doughera wrote: Show quoted text
> On Wed, Sep 20, 2017 at 06:54:20PM -0700, James E Keenan via RT wrote:
> > On Thu, 21 Sep 2017 00:47:45 GMT, tonyc wrote:
>
> > > minitest has ignored errors since it was introduced in > > > 16d20bd98cd29be76029ebf04027a7edd34d817b: > > > > > > commit 16d20bd98cd29be76029ebf04027a7edd34d817b > > > Author: Andy Dougherty <doughera@lafcol.lafayette.edu> > > > Date: Tue May 30 22:59:41 1995 +0000 > > > > > > This is my patch patch.1i for perl5.001. > > > > > > ... > > > Makefile.SH > > > ... > > > Add 'minitest' target. > > > > > > ... > > > diff --git a/Makefile.SH b/Makefile.SH > > > index f5dff60..cdd6333 100644 > > > --- a/Makefile.SH > > > +++ b/Makefile.SH > > > ... > > > @@ -374,6 +395,11 @@ test: miniperl perl preplibrary $(dynamic_ext) > > > - cd t && chmod +x TEST */*.t > > > - cd t && (rm -f perl; $(LNS) ../perl perl) && ./perl TEST > > > </dev/tty > > > > > > +minitest: miniperl > > > + - cd t && chmod +x TEST */*.t > > > + - cd t && (rm -f perl; $(LNS) ../miniperl perl) \ > > > + && ./perl TEST base/*.t comp/*.t cmd/*.t io/*.t > > > op/*.t > > > </dev/tty > > > + > > > clist: $(c) > > > echo $(c) | tr ' ' '\012' >.clist > > > > > > > > > Tony
> > > > So, on the one hand, I would argue, it does not DWIM for the purpose > > for which it is recommended in pod/perlgit.pod and by commenters on > > list today. > > > > On the other hand, to change that situation would require changing > > the way it has worked for 22 years (and I'm always inclined to trust > > Andy D's judgment). > > > > I'm inclined to the first hand. I want to use it in a string of > > commands and I want that string to fail if and when 'make minitest' > > fails. Otherwise I'm inclined to never use it.
> > While I appreciate the thought, I don't recall exercising any judgment > there. The 'minitest' target was pretty much a cut & paste of the > 'test' > target, which ignored the exit status. I don't recall why. Your > inclination > sounds sensible to me.
It turns out that having 'make' take the exit status of 'make minitest' into consideration is very simple; see patch attached. However, I want to re-read all references to 'minitest' in the core distro and to allow time for other people to comment. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 132139-minitest.diff
Download 132139-minitest.diff
text/x-patch 916b
diff --git a/Makefile.SH b/Makefile.SH index 730dcca..b0f0d9d 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -1631,15 +1631,15 @@ minitest_prep: @echo "You may see some irrelevant test failures if you have been unable" @echo "to build lib/Config.pm, or the Unicode data files." @echo " " - - cd t && (rm -f $(PERL_EXE); $(LNS) ../$(MINIPERL_EXE) $(PERL_EXE)) + cd t && (rm -f $(PERL_EXE); $(LNS) ../$(MINIPERL_EXE) $(PERL_EXE)) MINITEST_TESTS = base/*.t comp/*.t cmd/*.t run/*.t io/*.t re/*.t opbasic/*.t op/*.t uni/*.t perf/*.t minitest: $(MINIPERL_EXE) minitest_prep - - cd t && $(RUN_PERL) TEST $(MINITEST_TESTS) <$(devtty) + cd t && $(RUN_PERL) TEST $(MINITEST_TESTS) <$(devtty) minitest-notty minitest_notty: $(MINIPERL_EXE) minitest_prep - - cd t && PERL_SKIP_TTY_TEST=1 $(RUN_PERL) TEST $(MINITEST_TESTS) + cd t && PERL_SKIP_TTY_TEST=1 $(RUN_PERL) TEST $(MINITEST_TESTS) # Test via harness


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org