cygport test has zero exit status on failures

cygport test has zero exit status on failures

[Jason Pyeron]

Cygport test is not returning a non zero exit status on failure! Is this expected?

jenkinsoss-ci-cygwin ~/workspace/oss-cygwin-pdfgrep-2.1.2-x64
$ cygport pdfgrep.cygport test; echo $?
>>> Testing pdfgrep-2.1.2-1.x86_64
Making check in src
...
WARNING: Couldn't find the global config file.
Test Run By jenkins$ on Sun May  9 17:08:41 2021
Native configuration is x86_64-pc-cygwin

                === pdfgrep tests ===

Schedule of variations:
    unix

Running target unix
...
                === pdfgrep Summary ===

# of unexpected failures        2
# of unsupported tests          12

WARNING: pdfgrep_version failed:
terminate called after throwing an instance of 'std::runtime_error'
  what():  locale::facet::_S_create_c_locale name not valid
make[3]: *** [Makefile:445: check-DEJAGNU] Error 1
make[3]: Leaving directory '/cygdrive/c/Users/jenkins$/workspace/oss-cygwin-pdfgrep-2.1.2-x64/pdfgrep-2.1.2-1.x86_64/build/testsuite'
make[2]: *** [Makefile:545: check-am] Error 2
make[2]: Leaving directory '/cygdrive/c/Users/jenkins$/workspace/oss-cygwin-pdfgrep-2.1.2-x64/pdfgrep-2.1.2-1.x86_64/build/testsuite'
make[1]: *** [Makefile:352: check-recursive] Error 1
make[1]: Target 'check' not remade because of errors.
make[1]: Leaving directory '/cygdrive/c/Users/jenkins$/workspace/oss-cygwin-pdfgrep-2.1.2-x64/pdfgrep-2.1.2-1.x86_64/build/testsuite'
make[1]: Entering directory '/cygdrive/c/Users/jenkins$/workspace/oss-cygwin-pdfgrep-2.1.2-x64/pdfgrep-2.1.2-1.x86_64/build'
make[1]: Leaving directory '/cygdrive/c/Users/jenkins$/workspace/oss-cygwin-pdfgrep-2.1.2-x64/pdfgrep-2.1.2-1.x86_64/build'
make: *** [Makefile:389: check-recursive] Error 1
make: Target 'check' not remade because of errors.
0

Re: cygport test has zero exit status on failures

[ASSI]

Jason Pyeron writes:
> Cygport test is not returning a non zero exit status on failure! Is
> this expected?

Yes, it is using 'make -k' under the hood.  Again, if you need to do
something else than what the standard invocation provides, you would
generally write your own src_test() function.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

RE: cygport test has zero exit status on failures

[Jason Pyeron]

> -----Original Message-----
> From: ASSI
> Sent: Monday, May 10, 2021 1:06 AM
> 
> Jason Pyeron writes:
> > Cygport test is not returning a non zero exit status on failure! Is
> > this expected?
> 
> Yes, it is using 'make -k' under the hood.  Again, if you need to do
> something else than what the standard invocation provides, you would
> generally write your own src_test() function.

$ cat -n /usr/share/cygport/lib/src_test.cygpart
<snip/>
    33  cygtest() {
    34          if [ -e Makefile -o -e GNUmakefile -o -e makefile ]
    35          then
    36                  if make -n check &> /dev/null
    37                  then
    38                          make -k check || true
    39                  elif make -n test &> /dev/null
    40                  then
    41                          make -k test || true
    42                  else
    43                          inform "No testsuite detected.";
    44                  fi
    45          else
    46                  inform "No testsuite detected.";
    47          fi
    48  }
<snip/>

What is the historic rationale behind the "OR true" after the make check?

It seems silly to have to redefine src_test() as {
        cd ${B}
        make check
}, just to have a failure exit code if the test fail. It seems to have come in at https://github.com/cygwin/cygport/commit/ef93794f35de38b28f473b8f14c4a9e5a4a4a5a6 and never changed since. The commit does not indicate why the exit status is suppressed.

-Jason

Re: cygport test has zero exit status on failures

[ASSI]

Jason Pyeron writes:
> What is the historic rationale behind the "OR true" after the make
> check?

Not historic for the most part, I'd say.  Cygport can also do
cross-builds of packages and in those cases the tests will seldomly work
(at all or at least partly) unless upstream walked the extra mile.
Also, due to ATWIL syndrome and other factors, Cygwin is often not
explicitly considered a target platform or (even then) treated wrongly
in different ways, so even when building natively you will encounter
your fair share of spurious test fails.

> It seems silly to have to redefine src_test() as {
>         cd ${B}
>         make check
> }, just to have a failure exit code if the test fail.

It would be equally silly the other way around in a different set of
circumstances.  But yes, making this configurable might be useful, but
this should then be done in the local configuration, not in the cygport
file.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

RE: cygport test has zero exit status on failures

[Jason Pyeron]

> -----Original Message-----
> From: ASSI
> Sent: Tuesday, May 18, 2021 12:37 AM
> 
> Jason Pyeron writes:
> > What is the historic rationale behind the "OR true" after the make
> > check?
> 
> Not historic for the most part, I'd say.  Cygport can also do
> cross-builds of packages and in those cases the tests will seldomly work
> (at all or at least partly) unless upstream walked the extra mile.
> Also, due to ATWIL syndrome and other factors, Cygwin is often not

Which is why I want the tests to fail - so it causes patches to be created... and pushed upstream.

> explicitly considered a target platform or (even then) treated wrongly
> in different ways, so even when building natively you will encounter
> your fair share of spurious test fails.
> 
> > It seems silly to have to redefine src_test() as {
> >         cd ${B}
> >         make check
> > }, just to have a failure exit code if the test fail.
> 
> It would be equally silly the other way around in a different set of
> circumstances.  But yes, making this configurable might be useful, but
> this should then be done in the local configuration, not in the cygport
> file.

If I put it in ~/.config/cygport.conf it will impact all packages, not just the "one". The only place it is limited to applicable package is in the package.cygport file, or am I missing something?

e.g.

$ cat pdfgrep.cygport
NAME="pdfgrep"
VERSION=2.1.2
RELEASE=1
CATEGORY="Text"
SUMMARY="Command-line utility for searching text in PDFs"
DESCRIPTION="Pdfgrep is a tool to search text in PDF files. It works much
like grep, with one distinction: it operates on pages and not on lines."
HOMEPAGE="https://pdfgrep.org/"
SRC_URI="https://pdfgrep.org/download/pdfgrep-${VERSION}.tar.gz"

# git format-patch --stdout v2.1.2..cygwin-2.1.2 > v2.1.2..cygwin-2.1.2.patch
PATCH_URI="v2.1.2..cygwin-2.1.2.patch"

BUILD_REQUIRES="asciidoc gcc-g++ libpoppler-cpp-devel libgcrypt-devel libpcre-devel dejagnu texlive-collection-latex"

src_test() {
        cd ${B}
        make check
}

Re: cygport test has zero exit status on failures

[Achim Gratz]

Jason Pyeron writes:
> If I put it in ~/.config/cygport.conf it will impact all packages, not
> just the "one". The only place it is limited to applicable package is
> in the package.cygport file, or am I missing something?

You can always override these settings when starting a build on a CI
(either by changing the config file or just remove/set more environment
variables), but the point was that the cygport file should not lock in a
specific behaviour, but rather allow the environment to determine it.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

RE: cygport test has zero exit status on failures

[Jason Pyeron]

> -----Original Message-----
> From: Achim Gratz
> Sent: Tuesday, May 18, 2021 4:16 PM
> 
> Jason Pyeron writes:
> > If I put it in ~/.config/cygport.conf it will impact all packages, not
> > just the "one". The only place it is limited to applicable package is
> > in the package.cygport file, or am I missing something?
> 
> You can always override these settings when starting a build on a CI
> (either by changing the config file or just remove/set more environment
> variables), but the point was that the cygport file should not lock in a
> specific behaviour, but rather allow the environment to determine it.

Having trouble groking the conceptual design.

If I have a CI build for *.cygport - is it expected to have conditional logic based on which package is being processed? 

I thought was the purpose of the cygport file [1] was to contain only the compiling, testing, and installation instructions, just like portage [2]. 

In any case the all or all-test does not execute the test step, so the customization of the src_test does not impact the default behaviors.

As an example you can look at my CI builds [3] to see the sequence: 

1. ensure dependencies are installed on build host
2. cygport file.cygport download
3. cygport file.cygport all-test
4. cygport file.cygport test
5. archive artifacts

The same build host is used (serially) for other builds.

-Jason

1: /usr/share/doc/cygport/README §2
2: https://devmanual.gentoo.org/ebuild-writing/functions/src_test/index.html
3: https://www.pdinc.us/ci/job/oss-cygwin-pdfgrep/job/cygwin-cygport-2.1.2/3/

Re: cygport test has zero exit status on failures

[ASSI]

Jason Pyeron writes:
> If I have a CI build for *.cygport - is it expected to have
> conditional logic based on which package is being processed?

Not necessarily, I understand you were talking about default behaviour
and how the current default doesn't match your / your CI's expectation.
To reiterate, you can already do what you want by defining your own
src_test function and this behaviour becomes part of the package
definition then.

> I thought was the purpose of the cygport file [1] was to contain only
> the compiling, testing, and installation instructions, just like
> portage [2].

I consider cygport its own thing by now that is not actively
synchronized to what portage ends up doing.

> In any case the all or all-test does not execute the test step, so the
> customization of the src_test does not impact the default behaviors.

That's beside the point.  We were only talking about the test behaviours
or at least I was, anyway.  Now again, if you do that in the cygport
file you mix that expectation of getting a return value (that really
comes from the way you run cygport in the CI and determine the test
status) with the package definition.  If you force that to satisfy your
CI requirements, you also break the flow for folks who (as an example)
rather do

cygport $p finish prep compile inst test pkg

because a test fail would now error out before getting to the packaging
step.  Which is why I was musing that your preference in how to treat a
test fail should really be injected from the run-time environment rather
than the package definition.  Once that mechanism is in place I can also
start writing my own src_test functions that switch their behaviour
depending on that setting so that these package definitions would work
in either environment.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: cygport test has zero exit status on failures

[Marco Atzeri]

On 19.05.2021 07:16, ASSI wrote:
> Jason Pyeron writes:

> 
>> In any case the all or all-test does not execute the test step, so the
>> customization of the src_test does not impact the default behaviors.
> 
> That's beside the point.  We were only talking about the test behaviours
> or at least I was, anyway.  Now again, if you do that in the cygport
> file you mix that expectation of getting a return value (that really
> comes from the way you run cygport in the CI and determine the test
> status) with the package definition.  If you force that to satisfy your
> CI requirements, you also break the flow for folks who (as an example)
> rather do
> 
> cygport $p finish prep compile inst test pkg
> 
> because a test fail would now error out before getting to the packaging
> step.  Which is why I was musing that your preference in how to treat a
> test fail should really be injected from the run-time environment rather
> than the package definition.  Once that mechanism is in place I can also
> start writing my own src_test functions that switch their behaviour
> depending on that setting so that these package definitions would work
> in either environment.

in general a generic test failure is not a valid reason to NOT package.
Most of my packages have peculiar test failures that can be just ignored

In several, I just use "cygmake -i check" in src_test
to avoid premature stop of the tests.

Others requires to be installed before the testsuite is
properly executed.

> Regards,
> Achim.


Regards
Marco

RE: cygport test has zero exit status on failures

[Jason Pyeron]

> -----Original Message-----
> From: Marco Atzeri
> Sent: Wednesday, May 19, 2021 3:06 AM
> 
> On 19.05.2021 07:16, ASSI wrote:
> > Jason Pyeron writes:
> 
> >
> >> In any case the all or all-test does not execute the test step, so the
> >> customization of the src_test does not impact the default behaviors.
> >
> > That's beside the point.  We were only talking about the test behaviours
> > or at least I was, anyway.  Now again, if you do that in the cygport
> > file you mix that expectation of getting a return value (that really
> > comes from the way you run cygport in the CI and determine the test
> > status) with the package definition.  If you force that to satisfy your
> > CI requirements, you also break the flow for folks who (as an example)
> > rather do
> >
> > cygport $p finish prep compile inst test pkg

So for the pdfgrep that I am going to maintain, is it acceptable that I want the package to not happen until I address the test failure? Like so?

...
BUILD_REQUIRES="asciidoc gcc-g++ libpoppler-cpp-devel libgcrypt-devel libpcre-devel dejagnu texlive-collection-latex docbook-xml45"

src_test() {
        cd ${B}
        make check
}

> >
> > because a test fail would now error out before getting to the packaging
> > step.  Which is why I was musing that your preference in how to treat a
> > test fail should really be injected from the run-time environment rather
> > than the package definition.  Once that mechanism is in place I can also
> > start writing my own src_test functions that switch their behaviour
> > depending on that setting so that these package definitions would work
> > in either environment.
> 
> in general a generic test failure is not a valid reason to NOT package.
> Most of my packages have peculiar test failures that can be just ignored
> 
> In several, I just use "cygmake -i check" in src_test
> to avoid premature stop of the tests.
> 
> Others requires to be installed before the testsuite is
> properly executed.

Since it is not required to be installed, I would like to lean towards stopping to fix first.

v/r,

Jason

Re: cygport test has zero exit status on failures

[Achim Gratz]

Jason Pyeron writes:
> So for the pdfgrep that I am going to maintain, is it acceptable that
> I want the package to not happen until I address the test failure?

You are the maintainer.  :-)


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: cygport test has zero exit status on failures

[Marco Atzeri]

On 19.05.2021 18:32, Jason Pyeron wrote:
>> -----Original Message-----
>> From: Marco Atzeri
>> Sent: Wednesday, May 19, 2021 3:06 AM
>>
>> On 19.05.2021 07:16, ASSI wrote:
>>> Jason Pyeron writes:
>>
>>>
>>> cygport $p finish prep compile inst test pkg
> 
> So for the pdfgrep that I am going to maintain, is it acceptable that I want the package to not happen until I address the test failure? Like so?

you are the maintainer, it is your decision.
We just give you our opinion based on our experience

> ...
> BUILD_REQUIRES="asciidoc gcc-g++ libpoppler-cpp-devel libgcrypt-devel libpcre-devel dejagnu texlive-collection-latex docbook-xml45"
> 
> src_test() {
>          cd ${B}
>          make check
> }

This is the default.

>> in general a generic test failure is not a valid reason to NOT package.
>> Most of my packages have peculiar test failures that can be just ignored
>>
>> In several, I just use "cygmake -i check" in src_test
>> to avoid premature stop of the tests.
>>
>> Others requires to be installed before the testsuite is
>> properly executed.
> 
> Since it is not required to be installed, I would like to lean towards stopping to fix first.

The 1.4.1 seems fine. Or I am missing something ?

> 
> v/r,
> 
> Jason
> 

Regards
Marco

Re: cygport test has zero exit status on failures

[Achim Gratz]

Marco Atzeri via Cygwin-apps writes:
>> src_test() {
>>          cd ${B}
>>          make check
>> }
>
> This is the default.

The actual default is:

src_test() {
	cd ${B}
	cygtest
}

…with cygtest defined so that it won't return an error status from make.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds