Description
Jonathan Druart
2024-02-28 13:05:53 UTC
Created attachment 162530 [details] [review] Bug 36190: Adjust the test Created attachment 162531 [details] [review] Bug 36190: To not assume op is invalid if it is a TT variable Created attachment 163110 [details] [review] Bug 36190: To not assume op is invalid if it is a TT variable Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 163111 [details] [review] Bug 36190: (QA follow-up) Tidy Created attachment 163112 [details] [review] Bug 36190: Fix problems found by test opac-shelves - forms were stateless - changed to GET sco-main - forms stateless - changed to GET ** untested because sco + auth is broken smart-rules.tt - JS form template - added placeholder 'cud-' op ill-requests - added cud- tp ops ** tested comments, but not checkout, simple changes boraccount - removed repeated op - updated script patroncards/edit-batch - add placeholder 'cud-' op patroncards/manage - add placheholder 'cud-' op elasticsearch/mappings - separate forms - this could use a style follow-up, but makes more sense separate I think reports/dictionary - stateless - changed to GET guided_reports_start - stateless - changed to GET suggestion/suggestion - add placeholder 'cud-' op inventory - filed bug 36305, needs more handling I'm getting a sha1/fake ancestor error when trying to apply the patch: Applying: Bug 36190: To not assume op is invalid if it is a TT variable error: sha1 information is lacking or useless (xt/find-missing-op-in-forms.t). error: could not build fake ancestor Patch failed at 0001 Bug 36190: To not assume op is invalid if it is a TT variable ... Testing notes - how I planned to test: 1. Run the tests before the patch is applied: prove xt/find-missing-op-in-forms.t 2. Apply the first patch and run the tests: tests should fail 3. Apply the rest of the patches and run the tests: tests should pass! (In reply to David Nind from comment #6) > I'm getting a sha1/fake ancestor error when trying to apply the patch: Fixed, I obsoleted a patch without reattaching > > Testing notes - how I planned to test: > 1. Run the tests before the patch is applied: prove > xt/find-missing-op-in-forms.t > 2. Apply the first patch and run the tests: tests should fail > 3. Apply the rest of the patches and run the tests: tests should pass! Correct, except the test will have 1 remaining failure, should be handled on bug 36305 Created attachment 163149 [details] [review] Bug 36190: Adjust the test Signed-off-by: David Nind <david@davidnind.com> Created attachment 163150 [details] [review] Bug 36190: To not assume op is invalid if it is a TT variable Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 163151 [details] [review] Bug 36190: (QA follow-up) Tidy Signed-off-by: David Nind <david@davidnind.com> Created attachment 163152 [details] [review] Bug 36190: Fix problems found by test opac-shelves - forms were stateless - changed to GET sco-main - forms stateless - changed to GET ** untested because sco + auth is broken smart-rules.tt - JS form template - added placeholder 'cud-' op ill-requests - added cud- tp ops ** tested comments, but not checkout, simple changes boraccount - removed repeated op - updated script patroncards/edit-batch - add placeholder 'cud-' op patroncards/manage - add placheholder 'cud-' op elasticsearch/mappings - separate forms - this could use a style follow-up, but makes more sense separate I think reports/dictionary - stateless - changed to GET guided_reports_start - stateless - changed to GET suggestion/suggestion - add placeholder 'cud-' op inventory - filed bug 36305, needs more handling Signed-off-by: David Nind <david@davidnind.com> Thanks Nick! Testing notes (using KTD): 1. Run the tests before the patch is applied - tests should pass: prove -v xt/find-missing-op-in-forms.t 2. Apply the first three patches (using interactive mode) and run the tests: tests should fail with 12 forms missing the required information 3. Apply the remaining patch and run the tests again: tests still fail,[1] but this time there is only 1 form missing the required information (addressed in bug 36305) [1] After all patches are applied, only 1 form is missing the required information prove -v xt/find-missing-op-in-forms.t xt/find-missing-op-in-forms.t .. 1..1 not ok 1 - The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478) # Failed test 'The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)' # at xt/find-missing-op-in-forms.t line 43. # got: '1' # expected: '0' # $VAR1 = 'koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt:71,248'; # Looks like you failed 1 test of 1. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/1 subtests Test Summary Report ------------------- xt/find-missing-op-in-forms.t (Wstat: 256 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 1 Files=1, Tests=1, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.15 cusr 0.01 csys = 0.17 CPU) Result: FAIL Created attachment 163162 [details] [review] Bug 36190: Adjust the test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 163163 [details] [review] Bug 36190: To not assume op is invalid if it is a TT variable Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 163164 [details] [review] Bug 36190: (QA follow-up) Tidy Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 163165 [details] [review] Bug 36190: Fix problems found by test opac-shelves - forms were stateless - changed to GET sco-main - forms stateless - changed to GET ** untested because sco + auth is broken smart-rules.tt - JS form template - added placeholder 'cud-' op ill-requests - added cud- tp ops ** tested comments, but not checkout, simple changes boraccount - removed repeated op - updated script patroncards/edit-batch - add placeholder 'cud-' op patroncards/manage - add placheholder 'cud-' op elasticsearch/mappings - separate forms - this could use a style follow-up, but makes more sense separate I think reports/dictionary - stateless - changed to GET guided_reports_start - stateless - changed to GET suggestion/suggestion - add placeholder 'cud-' op inventory - filed bug 36305, needs more handling Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed for 24.05! Well done everyone, thank you! Depends on Bug 34478 not in 23.11.x |