Bug 24756 - Occasional failures for Koha/XSLT/Security.t
Summary: Occasional failures for Koha/XSLT/Security.t
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 23290
Blocks:
  Show dependency treegraph
 
Reported: 2020-02-28 08:18 UTC by Marcel de Rooy
Modified: 2020-04-13 17:01 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Bug 24756: Fix D8 and U18 failures for Koha/XSLT/Security.t (1.70 KB, patch)
2020-02-28 08:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: Skip last test for U18 (982 bytes, patch)
2020-02-28 10:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: Fix failing tests by removing temporary filenames (3.89 KB, patch)
2020-02-28 11:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: Show the warnings on Jenkins (1.39 KB, patch)
2020-03-02 09:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: Catch two failed to load-variations in Security.t (1.59 KB, patch)
2020-03-02 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: [19.11.X] Catch two failed to load-variations in Security.t (1.36 KB, patch)
2020-04-02 06:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24756: [19.11.X] Catch two failed to load-variations in Security.t (1.77 KB, patch)
2020-04-02 06:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2020-02-28 08:18:04 UTC
not ok 7 - Remote import does not fail on read_net

This occurs on D8 and U18 but not consistently?
For instance, at this very moment all Jenkins tests passed on D8 but master and 18.11 fail here for U18.
Unfortunately, I cant even reproduce the error on D8 and D9 in my test containers.

Trying to resolve it here by catching the warnings 'manually'.
Comment 1 Marcel de Rooy 2020-02-28 08:26:18 UTC
Created attachment 99746 [details] [review]
Bug 24756: Fix D8 and U18 failures for Koha/XSLT/Security.t

Instead of warning_like, we eventually catch multiple warnings and look
if we catch one specific warn and not catch another specific one.

Test plan:
Run t/db_dependent/Koha/XSLT/Security.t on D8, D9, D10 or U18.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Passes on D8 and D9 for me.
Comment 2 Marcel de Rooy 2020-02-28 08:33:23 UTC
Another option is to just remove this test. In the script we test the read_net case already. This is just something additional that we could miss too..
Comment 3 Martin Renvoize 2020-02-28 08:41:06 UTC
Skipping QA so we can see how Jenkins responds. Certainly, no harm will be done by this change :)
Comment 4 Martin Renvoize 2020-02-28 08:44:26 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 5 Marcel de Rooy 2020-02-28 09:42:25 UTC
U18 on master:

not ok 7 - Expected import error
ok 8 - No read_net warn for remote import
Comment 6 Marcel de Rooy 2020-02-28 10:23:42 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2020-02-28 10:24:11 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2020-02-28 10:27:30 UTC
Yeah, skip that skip. D9 failed for MDB?
Comment 9 Marcel de Rooy 2020-02-28 11:20:04 UTC
Created attachment 99754 [details] [review]
Bug 24756: Fix failing tests by removing temporary filenames

In order to resolve the random failure, that might be caused by reusing
filenames in File::Temp and caching xslt code, this patch replaces the
temporary file by using the code parameter.

Note: Since XSLT uses a digest based on the passed code fragment, we will
exclude collisions here.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2020-02-28 11:24:18 UTC
Martin,
Assuming that the failure might be in a combination of not unique file names and caching somehow, I added a second patch and tested it on D8, D9 and D10.
Hopefully catching it now..
Comment 11 Marcel de Rooy 2020-02-28 11:27:53 UTC
(In reply to Marcel de Rooy from comment #10)
> Martin,
> Assuming that the failure might be in a combination of not unique file names
> and caching somehow, I added a second patch and tested it on D8, D9 and D10.
> Hopefully catching it now..

Maybe I added the literal files once just for that reason. Who knows?
Comment 12 Marcel de Rooy 2020-03-02 08:52:45 UTC
Master D10 #100 29-02-2020 19:45 
not ok 7 - Expected import error
ok 8 - No read_net warn for remote import
Master D9 My8 #179 28-02-2020 15:57
idem not ok 7, ok 8
Comment 13 Marcel de Rooy 2020-03-02 09:03:25 UTC
Created attachment 99858 [details] [review]
Bug 24756: Show the warnings on Jenkins

Temporary measure to see which warnings Jenkins got while we cannot
reproduce them ourselves on D8, D9 and D10.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2020-03-02 09:04:49 UTC
This produces output (in my D9 testing environment) like:

ok 7 - Expected import error. Additional info: $VAR1 = 'XSLT::Base: No such file or directoryI/O warning : failed to load external entity "http://notexpected.koha-community.org/noxsl/nothing.xsl"
# compilation error: file unknown-558d811413c0 element import
# xsl:import : unable to load http://notexpected.koha-community.org/noxsl/nothing.xsl
#  at /usr/share/koha/Koha/XSLT/Base.pm line 343.
# ';
#
ok 8 - No read_net warn for remote import

Hopefully we'll know more when we run this on Jenkins?
Comment 15 Marcel de Rooy 2020-03-02 11:01:30 UTC
(In reply to Marcel de Rooy from comment #14)
> Hopefully we'll know more when we run this on Jenkins?

=== https://jenkins.koha-community.org/job/Koha_Master_D8/725/
not ok 7 - Expected import error. Additional info: $VAR1 = 'XSLT::Base: I/O warning : failed to load HTTP resource
# compilation error: file unknown-3d22ae0 element import
# xsl:import : unable to load http://notexpected.koha-community.org/noxsl/nothing.xsl
#  at /kohadevbox/koha/Koha/XSLT/Base.pm line 343.
# ';
# 
ok 8 - No read_net warn for remote import

Here the wording "failed to load HTTP resource" is used instead of "failed to load external entity". It appears that both can come up.
libxml2 contains both strings
  failed to load external entity "%s"
  failed to load HTTP resource "%s"
and even:
  Failed to load externalRef %s
  Failed to load include %s
(But we did not see the last two.)

So we probably just need:
    is( ( grep { /failed to load (external entity|HTTP resource)/ } @warn ), 1, 'Expected import error' );

I will wait a few days now before replacing the additional Dumper info.
Comment 16 Marcel de Rooy 2020-03-02 11:16:54 UTC
Created attachment 99892 [details] [review]
Bug 24756: Catch two failed to load-variations in Security.t

Test 7 in Security.t failed when 'failed to load HTTP resource' was
returned by libxml2 instead of 'failed to load external entity'.

We now remove the debugging with Dumper again and adjust the regex to
include both variations.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Martin Renvoize 2020-03-03 10:38:40 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 18 Jonathan Druart 2020-03-24 09:43:15 UTC
It seems that we need it for 19.11 as well

19.11_D9 (79)

koha_1       | #   Failed test 'Remote import does not fail on read_net'
koha_1       | #   at t/db_dependent/Koha/XSLT/Security.t line 124.
koha_1       | # found warning: XSLT_Handler: I/O warning : failed to load HTTP resource
koha_1       | # expected to find warning: (?^u:I/O warning : failed to load external entity)
koha_1       | # Looks like you failed 1 test of 7.
Comment 19 Marcel de Rooy 2020-03-24 13:22:08 UTC
(In reply to Jonathan Druart from comment #18)
> It seems that we need it for 19.11 as well
> 
> 19.11_D9 (79)
> 
> koha_1       | #   Failed test 'Remote import does not fail on read_net'
> koha_1       | #   at t/db_dependent/Koha/XSLT/Security.t line 124.
> koha_1       | # found warning: XSLT_Handler: I/O warning : failed to load
> HTTP resource
> koha_1       | # expected to find warning: (?^u:I/O warning : failed to load
> external entity)
> koha_1       | # Looks like you failed 1 test of 7.

This should be backported, yes. The RMaints look in this queue, right?
Comment 20 Joy Nelson 2020-04-01 20:08:56 UTC
this does not apply cleanly to 19.11.x branch.  Please rebase. 
Thanks!
Comment 21 Marcel de Rooy 2020-04-02 06:19:57 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2020-04-02 06:20:44 UTC
(In reply to Joy Nelson from comment #20)
> this does not apply cleanly to 19.11.x branch.  Please rebase. 
> Thanks!

Hi Joy.
Actually you only need the last patch marked 19.11.X.
Comment 23 Marcel de Rooy 2020-04-02 06:36:18 UTC
Created attachment 102271 [details] [review]
Bug 24756: [19.11.X] Catch two failed to load-variations in Security.t

Test 7 in Security.t failed when 'failed to load HTTP resource' was
returned by libxml2 instead of 'failed to load external entity'.

We now remove the debugging with Dumper again and adjust the regex to
include both variations.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Joy Nelson 2020-04-03 20:06:09 UTC
backported 19.11 patch to 19.11.x branch for 19.11.05
Comment 25 Lucas Gass 2020-04-13 17:01:51 UTC
backported to 19.05.x for 19.05.10