Description
Mark Tompsett
2017-01-16 19:24:49 UTC
Created attachment 59026 [details] [review] Bug 17917: Fix Jenkins tests failing on C4::Member. C4/Members.pm line 502 references: C4::Context->userenv->{'branch'}. As the tests are not logged in userenv is not defined, and triggers test failures. This patches it by giving a reasonable branch value. Test Plan --------- 1) prove t/Members/cardnumber.t -- fails 2) apply patch 3) prove t/Members/cardnumber.t -- passes 4) run koha qa test tools Created attachment 59027 [details] [review] Bug 17917: Fix Jenkins tests failing on C4::Member. C4/Members.pm line 502 references: C4::Context->userenv->{'branch'}. As the tests are not logged in userenv is not defined, and triggers test failures. This patches it by giving a reasonable branch value. Test Plan --------- 1) prove t/db_dependent/Holds.t -- fails 2) apply patch 3) prove t/db_dependent/Holds.t -- passes 4) run koha qa test tools Created attachment 59028 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Holds.t This changes AddMember into TestBuilder calls. TEST PLAN --------- 1) prove -v t/db_dependent/Holds.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Holds.t -- works 4) run koha qa test tools Created attachment 59029 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Members/IssueSlip.t TEST PLAN --------- 1) prove -v t/db_dependent/Members/IssueSlip.t -- fail 2) apply this patch 3) prove -v t/db_dependent/Members/IssueSlip.t -- success 4) run koha qa test tools Created attachment 59030 [details] [review] Bug 17917: Fix Jenkinds for t/db_dependent/Holds/LocalHoldsPriority.t TEST PLAN --------- 1) prove -v t/db_dependent/Holds/LocalHoldsPriority.t -- fail 2) apply this patch 3) prove -v t/db_dependent/Holds/LocalHoldsPriority.t --success 4) run koha qa test tools Created attachment 59031 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Letters.t TEST PLAN --------- 1) prove -v t/db_dependent/Letters.t -- fail 2) apply this patch 3) prove -v t/db_dependent/Letters.t -- success 4) run koha qa test tools Created attachment 59039 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Circulation/issue.t TEST PLAN --------- 1) prove -v t/db_dependent/Circulation/issue.t -- fail 2) apply this patch 3) prove -v t/db_dependent/Circulation/issue.t -- success 4) run koha qa test tools Created attachment 59040 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Circulation/MarkIssueReturned.t TEST PLAN --------- 1) prove -v t/db_dependent/Circulation/MarkIssueReturned.t -- fails 2) apply rhis patch 3) prove -v t/db_dependent/Circulation/MarkIssueReturned.t -- succeeds 4) run koha qa test tools Created attachment 59041 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Members/GetAllIssues.t TEST PLAN --------- 1) prove -v t/db_dependent/Members/GetAllIssues.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Members/GetAllIssues.t -- succeeds 4) run koha qa test tools Created attachment 59042 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Serials_2.t TEST PLAN --------- 1) prove -v t/db_dependent/Serials_2.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Serials_2.t -- succeeds 4) run koha qa test tools Created attachment 59044 [details] [review] Bug 17917: Fix Jenkin for t/db_dependent/Members/GetOverdues.t TEST PLAN --------- 1) prove -v t/db_dependent/Members/GetOverdues.t - fails 2) apply this patch 3) prove -v t/db_dependent/Members/GetOverdues.t 4) run koha qa test tools Created attachment 59045 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Members/GetPendingIssues.t TEST PLAN --------- 1) prove -v t/db_dependent/Members/GetPendingIssues.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Members/GetPendingIssues.t -- succeeds 4) run koha qa test tools Created attachment 59046 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Circulation/Branch.t TEST PLAN --------- 1) prove -v t/db_dependent/Circulation/Branch.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Circulation/Branch.t -- succeeds 4) run koha qa test tools Created attachment 59052 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Reserves.t TEST PLAN --------- 1) prove -v t/db_dependent/Reserves.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Reserves.t -- succeeds 4) run koha qa test tools Most of the issues were AddMember calls failing. Those are the one's intended to be fixed by this bug. Strange thing is I can't replicate on kohadevbox, but I can on my git install. Created attachment 59062 [details] [review] Bug 17917: fix Jenkins for t/db_dependent/Suggestions.t TEST PLAN --------- 1) sudo koha-shell -c "prove t/db_dependent/Suggestions.t" kohadev -- fails 2) apply patch 3) sudo koha-shell -c "prove t/db_dependent/Suggestions.t" kohadev -- succeeds 4) run koha qa test tools This is wrong, see the last patch on bug 6782. It is not pushed yet, but I have asked Kyle to push it quickly. You should always take a look at the patch that introduces the regression, I usually attach the patch on it (if it's trivial) or create another and link it. Why do you want to replace AddMember calls with TestBuilder? Created attachment 59175 [details] [review] Bug 17917: Fix Jenkins for t/db_dependent/Holds.t This changes AddMember into TestBuilder calls. TEST PLAN --------- 1) prove -v t/db_dependent/Holds.t -- fails 2) apply this patch 3) prove -v t/db_dependent/Holds.t -- works 4) run koha qa test tools (In reply to Jonathan Druart from comment #19) > Why do you want to replace AddMember calls with TestBuilder? 1) Because it fixes the failed tests. 2) Because none of the tests actually are testing whether a member was added correctly or not. (i.e. not t/db_dependent/Members.t) 3) Because putting in the userenv logic method seemed like the wrong fix compared to switching to TestBuilder. 4) Because TestBuilder generates a nicer patron record than the userenv logic that would have been added. 5) even with your change from bug 6782, there are still failures. TEST PLAN --------- vagrant up jessie cd kohaclone git checkout -b bug_17917 origin/master git bz apply 17917 y git checkout master sudo koha-shell -c "prove t/db_dependent" kohadev # This has faillures git checkout bug_17917 sudo koha-shell -c "prove t/db_dependent" kohadev # This drastically reduces the problems. Still kind of bummed about check_sysprefs failing when t/db_dependent, but not when individually. And, Reserves.t needs a patch from 13113 applied, because it got missed. (In reply to M. Tompsett from comment #21) > And Reserves.t needs a patch from 13113 applied, > because it got missed. Or my lack of sleep made me misread code. but some how it is failing too. I have tested this patch and the second Koha-shell command does give a distractingly less amount of failures, however there are still many failures: This is the end result of the test plan Test Summary Report ------------------- t/db_dependent/00-strict.t (Wstat: 256 Tests: 661 Failed: 1) Failed test: 368 Non-zero exit status: 1 t/db_dependent/Koha_Elasticsearch_Indexer.t (Wstat: 65280 Tests: 2 Failed: 1) Failed test: 1 Non-zero exit status: 255 Parse errors: Bad plan. You planned 5 tests but ran 2. t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t (Wstat: 2816 Tests: 11 Failed: 2) Failed tests: 1, 11 Non-zero exit status: 11 t/db_dependent/Reserves.t (Wstat: 65280 Tests: 52 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 72 tests but ran 52. t/db_dependent/Sitemapper.t (Wstat: 65280 Tests: 3 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 16 tests but ran 3. t/db_dependent/TestBuilder.t (Wstat: 256 Tests: 10 Failed: 1) Failed test: 3 Non-zero exit status: 1 t/db_dependent/check_sysprefs.t (Wstat: 256 Tests: 2 Failed: 1) Failed test: 1 Non-zero exit status: 1 Files=87, Tests=3228, 1056 wallclock secs ( 0.98 usr 0.29 sys + 960.02 cusr 58.44 csys = 1019.73 CPU) Result: FAIL Here is an test run of mine, demonstrating only two known issues, which are technically beyond the AddMember scope of the other ones. I would be willing to open another bug for those, if these would Pass QA. vagrant@kohadevbox:kohaclone(bug_17917)$ git fetch [My guess is the latest master wasn't pulled down when you applied and tested these] vagrant@kohadevbox:kohaclone(bug_17917)$ git checkout -b bug_17917_v2 origin/master Branch bug_17917_v2 set up to track remote branch master from origin. Switched to a new branch 'bug_17917_v2' vagrant@kohadevbox:kohaclone(bug_17917_v2)$ git bz apply 17920 Bug 17920 - t/db_dependent/Sitemapper.t fails because of permissions 59157 - Bug 17920: t/db_dependent/Sitemapper.t fails because of permissions Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17920: t/db_dependent/Sitemapper.t fails because of permissions [This is because Sitemapper is a permissions issue, and not an AddMember issue] vagrant@kohadevbox:kohaclone(bug_17917_v2)$ git bz apply 17917 Bug 17917 - Jenkins tests failing because test suite not logged in user 59029 - Bug 17917: Fix Jenkins for t/db_dependent/Members/IssueSlip.t 59030 - Bug 17917: Fix Jenkinds for t/db_dependent/Holds/LocalHoldsPriority.t 59031 - Bug 17917: Fix Jenkins for t/db_dependent/Letters.t 59039 - Bug 17917: Fix Jenkins for t/db_dependent/Circulation/issue.t 59040 - Bug 17917: Fix Jenkins for t/db_dependent/Circulation/MarkIssueReturned.t 59041 - Bug 17917: Fix Jenkins for t/db_dependent/Members/GetAllIssues.t 59042 - Bug 17917: Fix Jenkins for t/db_dependent/Serials_2.t 59044 - Bug 17917: Fix Jenkin for t/db_dependent/Members/GetOverdues.t 59045 - Bug 17917: Fix Jenkins for t/db_dependent/Members/GetPendingIssues.t 59046 - Bug 17917: Fix Jenkins for t/db_dependent/Circulation/Branch.t 59052 - Bug 17917: Fix Jenkins for t/db_dependent/Reserves.t 59062 - Bug 17917: fix Jenkins for t/db_dependent/Suggestions.t 59175 - Bug 17917: Fix Jenkins for t/db_dependent/Holds.t Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17917: Fix Jenkins for t/db_dependent/Members/IssueSlip.t Applying: Bug 17917: Fix Jenkinds for t/db_dependent/Holds/LocalHoldsPriority.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Letters.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Circulation/issue.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Circulation/MarkIssueReturned.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Members/GetAllIssues.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Serials_2.t Applying: Bug 17917: Fix Jenkin for t/db_dependent/Members/GetOverdues.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Members/GetPendingIssues.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Circulation/Branch.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Reserves.t Applying: Bug 17917: fix Jenkins for t/db_dependent/Suggestions.t Applying: Bug 17917: Fix Jenkins for t/db_dependent/Holds.t vagrant@kohadevbox:kohaclone(bug_17917_v2)$ sudo koha-shell -c "prove t/db_dependent" kohadev [SNIP ok's] t/db_dependent/FrameworkPlugin.t ......................... 1/5 oAuth error: Database unavailable (109) authorities Bib-1 t/db_dependent/FrameworkPlugin.t ......................... ok [This bug is about AddMember triggered errors] [SNIP ok's] t/db_dependent/ILSDI_Services.t .......................... 1/3 Odd number of elements in anonymous hash at /usr/share/koha/lib/C4/ILSDI/Services.pm line 379. Use of uninitialized value in numeric gt (>) at /usr/share/koha/lib/C4/ILSDI/Services.pm line 379. t/db_dependent/ILSDI_Services.t .......................... ok [This is fixed by 17836] [SNIP ok's] t/db_dependent/Koha_Authority.t .......................... 1/? Use of uninitialized value $authid in addition (+) at /usr/share/koha/lib/C4/AuthoritiesMarc.pm line 672, <GEN26> line 1. t/db_dependent/Koha_Authority.t .......................... ok [SNIP ok's] t/db_dependent/Reserves.t ................................ 8/72 Use of uninitialized value $tagno in concatenation (.) or string at /usr/share/perl5/MARC/Field.pm line 83. Tag "" is not a valid tag. at t/db_dependent/Reserves.t line 544. # Looks like your test exited with 255 just after 52. t/db_dependent/Reserves.t ................................ Dubious, test returned 255 (wstat 65280, 0xff00) Failed 20/72 subtests [Still not solved yet] [SNIP ok's] t/db_dependent/Search.t .................................. The syspref casAuthentication was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 140. The syspref casLogout was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 140. The syspref casServerUrl was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 140. The syspref template was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 140. ERROR DECODING RECORD - Tag "50%" is not a valid tag. at /usr/share/perl5/MARC/File/USMARC.pm line 223. : 00295nam a22001217a 450000300040000000500170000400800410002110000180006224500570008050%001100137942001200148999001300160OSt20111207151138.0111207t xxu||||| |||| 00| 0 eng d aGanser, Helen aA record containing a punctuation character as a tag aa note 2ddccBK c214d214 at /usr/share/koha/lib/C4/Search.pm line 680. t/db_dependent/Search.t .................................. 1/4 The syspref template was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 140. t/db_dependent/Search.t .................................. ok [This bug is about AddMember triggered errors] [SNIP ok's] t/db_dependent/check_sysprefs.t .......................... readline() on closed filehandle $ref_fh at t/db_dependent/check_sysprefs.t line 57. t/db_dependent/check_sysprefs.t .......................... 1/? # Failed test 'Found 0 sysprefs' # at t/db_dependent/check_sysprefs.t line 40. # '-1' # >= # '0' # Looks like you failed 1 test of 2. t/db_dependent/check_sysprefs.t .......................... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests [Still not solved yet. Something about intranet config being a /tmp/... directory] [SNIP ok's] Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 65280 Tests: 52 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 72 tests but ran 52. t/db_dependent/check_sysprefs.t (Wstat: 256 Tests: 2 Failed: 1) Failed test: 1 Non-zero exit status: 1 Files=87, Tests=3244, 618 wallclock secs ( 0.66 usr 0.07 sys + 561.44 cusr 28.40 csys = 590.57 CPU) Result: FAIL [As expected just these two fails, because they are not AddMember issues] [However, it would be nice to fix them here] Created attachment 59206 [details] [review] Bug 17917: Fix Cache issue with t/db_dependent/Reserves.t Forcing a cache flush makes the age restriction tests work. TEST PLAN --------- $ sudo koha-shell -c "prove t/db_dependent/Reserves.t" kohadev -- fails -- apply this patch $ sudo koha-shell -c "prove t/db_dependent/Reserves.t" kohadev -- succeeds -- run koha qa test tools Created attachment 59207 [details] [review] Bug 17917: Fix path issue in t/db_dependent/check_sysprefs.t C4::Context->config('intranet') is returning different values based on whether it is a full t/db_dependent test or a specific check_sysprefs.t test run. Tests are only run in kohaclone, so it makes no sense to reference config at all. Brought in rel2abs logic to build a reasonable $root_dir. TEST PLAN --------- git checkout master git pull sudo koha-shell -c "prove t/db_dependent" kohadev -- this should have a check_sysprefs.t failure git checkout -b bug_17917 origin/master git bz apply 17920 git bz apply 17917 sudo koha-shell -c "prove t/db_dependent" kohadev -- There should be no issues. -- If there are, it should be intermittent. -- run koha qa test tools I don't understand why you remove the AddMember calls. AddMember does much more than inserting a row in the borrowers using TestBuilder. Looking at trimming some of my overzealous efforts. :) Created attachment 59274 [details] [review] Bug 17917: Fix path issue in t/db_dependent/check_sysprefs.t C4::Context->config('intranet') is returning different values based on whether it is a full t/db_dependent test or a specific check_sysprefs.t test run. Tests are only run in kohaclone, so it makes no sense to reference config at all. Brought in rel2abs logic to build a reasonable $root_dir. TEST PLAN --------- -- on a kohadevbox sudo apt-get update sudo apt-get upgrade -- need to make sure the koha-commonpackage is more current git checkout master git pull sudo koha-shell -c "prove t/db_dependent" kohadev -- this should have a check_sysprefs.t failure sudo koha-shell -c "prove t/db_dependent/check_sysprefs.t" kohadev -- this should strangely pass git checkout -b bug_17917 origin/master git bz apply 17917 sudo koha-shell -c "prove t/db_dependent" kohadev -- there will not be a check_sysprefs.t failure sudo koha-shell -c "prove t/db_dependent/check_sysprefs.t" kohadev -- this should pass -- run koha qa test tools Created attachment 59287 [details] [review] Bug 17917: Fix path issue in t/db_dependent/check_sysprefs.t C4::Context->config('intranet') is returning different values based on whether it is a full t/db_dependent test or a specific check_sysprefs.t test run. Tests are only run in kohaclone, so it makes no sense to reference config at all. Brought in rel2abs logic to build a reasonable $root_dir. TEST PLAN --------- -- on a kohadevbox sudo apt-get update sudo apt-get upgrade -- need to make sure the koha-commonpackage is more current git checkout master git pull sudo koha-shell -c "prove t/db_dependent" kohadev -- this should have a check_sysprefs.t failure sudo koha-shell -c "prove t/db_dependent/check_sysprefs.t" kohadev -- this should strangely pass git checkout -b bug_17917 origin/master git bz apply 17917 sudo koha-shell -c "prove t/db_dependent" kohadev -- there will not be a check_sysprefs.t failure sudo koha-shell -c "prove t/db_dependent/check_sysprefs.t" kohadev -- this should pass -- run koha qa test tools Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> https://bugs.koha-community.org/show_bug.cgi?id=16387 Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Can reproduce the problem. But I think that we want to know why test fails under prove, instead of just building the path some other way. We should be able to rely on config You say: "C4::Context->config('intranet') is returning different values ..." Could you detail what these different values are? And can we reconstruct where they come from? I checked a few combinations of "prove [some_tests] check_syspref" but could not find the trigger yet. Parking in Blocked for the moment. Problems are caused by Search.t (still looking a bit) Created attachment 59299 [details] [review] Bug 17917: [QA Follow-up] Reprove Search.t The config tweaks from Search.t break check_sysprefs.t later on. Clearing the cache resolves that. Test plan: [1] Run prove t/db_dependent/Search.t t/db_dependent/check_sysprefs.t (in one statement) before and after applying this patch. Created attachment 59310 [details] [review] Bug 17917: [QA Follow-up] Reprove Search.t The config tweaks from Search.t break check_sysprefs.t later on. Clearing the cache resolves that. Test plan: [1] Run prove t/db_dependent/Search.t t/db_dependent/check_sysprefs.t (in one statement) before and after applying this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 59311 [details] [review] Bug 17917: [AMENDED] Fix path issue in t/db_dependent/check_sysprefs.t EDIT (from Marcel:) Previous patch fixes issue caused by Search.t. This patch still adds some small changes to check_sysprefs.t. Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 59313 [details] [review] Bug 17917: [AMENDED] Fix path issue in t/db_dependent/check_sysprefs.t EDIT (from Marcel): Previous patch fixes issue caused by Search.t. This patch still adds some small changes to check_sysprefs.t. Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 59319 [details] [review] Bug 7143: [QA Follow-up] Handling tabs Editing spaces before releases and developer: 876,$s/ \+releases$/^Ireleases/ (20 substitutions) 876,$s/ \+developer$/^Ideveloper/ (7 substitutions on 7 lines) Editing spaces between date and text: 876,$s/2016 \+/2016^I/ 876,$s/2017 \+/2017^I/ Fixed a few single spaces too. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Display on Timeline tab looks good now. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 59320 [details] [review] Bug 17917: [QA Follow-up] Reprove Search.t The config tweaks from Search.t break check_sysprefs.t later on. Clearing the cache resolves that. Test plan: [1] Run prove t/db_dependent/Search.t t/db_dependent/check_sysprefs.t (in one statement) before and after applying this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> A prove t/db_dependent did not fail check_sysprefs.t (only skipping two tests: 00-strict.t and Koha/IssuingRules.t; not enough patience) Comment on attachment 59319 [details] [review] Bug 7143: [QA Follow-up] Handling tabs Making a mess here Created attachment 59321 [details] [review] Bug 17917: [AMENDED] Fix path issue in t/db_dependent/check_sysprefs.t EDIT (from Marcel): Previous patch fixes issue caused by Search.t. This patch still adds some small changes to check_sysprefs.t. Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 17.05, thanks Marcel, Mark! These patches have been pushed to 16.11.x and will be in 16.11.02. . Pushed to 16.05.x, for 16.05.08 release (In reply to Mason James from comment #43) > Pushed to 16.05.x, for 16.05.08 release Update - I reverted this patchset from 16.05.x after hitting failing test from t/db_dependent/Search.t transcript below.. $ prove t/db_dependent/Search.t ... t/db_dependent/Search.t .. 4/4 Can't locate object method "get_instance" via package "Koha::Caches" (perhaps you forgot to load "Koha::Caches"?) at t/db_dependent/Search.t line 971. # Looks like your test exited with 255 just after 4. t/db_dependent/Search.t .. Dubious, test returned 255 (wstat 65280, 0xff00) All 4 subtests passed Test Summary Report ------------------- t/db_dependent/Search.t (Wstat: 65280 Tests: 4 Failed: 0) Non-zero exit status: 255 Files=1, Tests=4, 34 wallclock secs ( 0.05 usr 0.02 sys + 12.82 cusr 9.46 csys = 22.35 CPU) Result: FAIL |