Bug 24760 - BackgroundJob tests fail with latest versions of YAML or YAML::Syck
Summary: BackgroundJob tests fail with latest versions of YAML or YAML::Syck
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-28 13:11 UTC by Julian Maurice
Modified: 2020-04-20 00:02 UTC (History)
4 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 24760: Use C4::BackgroundJob->fetch in tests (2.55 KB, patch)
2020-02-28 13:37 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 24760: Use C4::BackgroundJob->fetch in tests (2.60 KB, patch)
2020-03-02 22:55 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 24760: Use C4::BackgroundJob->fetch in tests (2.70 KB, patch)
2020-03-06 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24760: (follow-up) Add a neighboring test messsage (1.11 KB, patch)
2020-03-06 11:16 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 Julian Maurice 2020-02-28 13:11:58 UTC
YAML 1.30 and YAML::Syck 1.32 introduce a breaking change : they won't bless loaded objects by default, and t/db_dependent/BackgroundJob.t fail when this feature is disabled.

I don't know this module very well, but I think that the tests are broken.
C4::BackgroundJob::fetch already bless the object returned from $session->param, so we don't need to test that $session->param returns a blessed object.
Comment 1 Julian Maurice 2020-02-28 13:37:08 UTC
Created attachment 99756 [details] [review]
Bug 24760: Use C4::BackgroundJob->fetch in tests

Starting with YAML 1.30 and YAML::Syck 1.32, these modules don't bless
loaded objects by default. This is not a problem as
C4::BackgroundJob->fetch bless them anyway.

So, instead of testing what's in the session, test the result of
C4::BackgroundJob->fetch, which is what's used everywhere else

Test plan:
1. Install latest version of YAML::Syck (or YAML if YAML::Syck is not
installed)
2. prove t/db_dependent/BackgroundJob.t => should FAIL
3. Apply patch
4. prove t/db_dependent/BackgroundJob.t => should PASS
5. Verify that background jobs are still working (for instance,
tools/batchMod.pl)
Comment 2 Michal Denar 2020-03-02 22:55:17 UTC
Created attachment 99968 [details] [review]
Bug 24760: Use C4::BackgroundJob->fetch in tests

Starting with YAML 1.30 and YAML::Syck 1.32, these modules don't bless
loaded objects by default. This is not a problem as
C4::BackgroundJob->fetch bless them anyway.

So, instead of testing what's in the session, test the result of
C4::BackgroundJob->fetch, which is what's used everywhere else

Test plan:
1. Install latest version of YAML::Syck (or YAML if YAML::Syck is not
installed)
2. prove t/db_dependent/BackgroundJob.t => should FAIL
3. Apply patch
4. prove t/db_dependent/BackgroundJob.t => should PASS
5. Verify that background jobs are still working (for instance,
tools/batchMod.pl)

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 3 Marcel de Rooy 2020-03-06 10:54:25 UTC
QAing
Comment 4 Marcel de Rooy 2020-03-06 11:16:08 UTC
Created attachment 100248 [details] [review]
Bug 24760: Use C4::BackgroundJob->fetch in tests

Starting with YAML 1.30 and YAML::Syck 1.32, these modules don't bless
loaded objects by default. This is not a problem as
C4::BackgroundJob->fetch bless them anyway.

So, instead of testing what's in the session, test the result of
C4::BackgroundJob->fetch, which is what's used everywhere else

Test plan:
1. Install latest version of YAML::Syck (or YAML if YAML::Syck is not
installed)
2. prove t/db_dependent/BackgroundJob.t => should FAIL
3. Apply patch
4. prove t/db_dependent/BackgroundJob.t => should PASS
5. Verify that background jobs are still working (for instance,
tools/batchMod.pl)

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2020-03-06 11:16:12 UTC
Created attachment 100249 [details] [review]
Bug 24760: (follow-up) Add a neighboring test messsage

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Martin Renvoize 2020-03-06 15:02:42 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 7 Joy Nelson 2020-04-02 16:42:27 UTC
Backported to 19.11.x for 19.11.05
Comment 8 Lucas Gass 2020-04-09 20:45:51 UTC
backported to 19.05.x for 19.05.10
Comment 9 Hayley Mapley 2020-04-20 00:02:30 UTC
Normal patch will not be backported to 18.11.x series