Bug 28483 - Warnings from Search.t must be removed
Summary: Warnings from Search.t must be removed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Nick Clemens
URL:
Keywords:
Depends on: 21260
Blocks: 25515
  Show dependency treegraph
 
Reported: 2021-05-28 12:30 UTC by Jonathan Druart
Modified: 2022-06-06 20:25 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.02


Attachments
Bug 28483: Avoid useless warnings (2.97 KB, patch)
2021-06-23 08:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28483: Remove warnings from Search.t (2.98 KB, patch)
2021-06-23 08:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28483: Remove warnings from Search.t (3.03 KB, patch)
2021-07-09 12:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28483: Fix t/Search/buildQuery.t (2.35 KB, patch)
2021-07-13 07:47 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-05-28 12:30:55 UTC
The syspref Reference_NFL_Statuses was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 197.
Comment 1 Jonathan Druart 2021-06-23 08:01:45 UTC
Created attachment 122312 [details] [review]
Bug 28483: Avoid useless warnings

There are too many warnings about uninitialized variables in Search.pm.
This patch deals with that, the same way it is dealt accross the file:
by setting an empty string when things are not defined.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Search.t
=> FAIL: Wow, too many warnings. Some related to this bug, some not.
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more warnings!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 2 Jonathan Druart 2021-06-23 08:02:52 UTC
Created attachment 122313 [details] [review]
Bug 28483: Remove warnings from Search.t

There are too many warnings about uninitialized variables in Search.pm.
This patch deals with that, the same way it is dealt accross the file:
by setting an empty string when things are not defined.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Search.t
=> FAIL: Wow, too many warnings. Some related to this bug, some not.
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more warnings!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Marcel de Rooy 2021-06-23 14:55:22 UTC
When seeing this, I wonder right away why not the other one?

-    my $query        = $operands[0];
+    my $query        = $operands[0] // "";
     my $simple_query = $operands[0];

And looking a bit later, I see:
    for ($query_cgi,$simple_query) {
        s/"//g;
    }
Shouldnt this trigger a warn too when undefined?
Comment 4 Nick Clemens 2021-07-09 12:46:51 UTC
Created attachment 122740 [details] [review]
Bug 28483: Remove warnings from Search.t

There are too many warnings about uninitialized variables in Search.pm.
This patch deals with that, the same way it is dealt accross the file:
by setting an empty string when things are not defined.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Search.t
=> FAIL: Wow, too many warnings. Some related to this bug, some not.
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more warnings!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens 2021-07-09 12:47:35 UTC
(In reply to Marcel de Rooy from comment #3)
> When seeing this, I wonder right away why not the other one?
> 
> -    my $query        = $operands[0];
> +    my $query        = $operands[0] // "";
>      my $simple_query = $operands[0];
> 
> And looking a bit later, I see:
>     for ($query_cgi,$simple_query) {
>         s/"//g;
>     }
> Shouldnt this trigger a warn too when undefined?

The code sets it undef later, and simple_query is not used between the two places
Comment 6 Marcel de Rooy 2021-07-12 09:12:53 UTC
(In reply to Nick Clemens from comment #5)
> (In reply to Marcel de Rooy from comment #3)
> > When seeing this, I wonder right away why not the other one?
> > 
> > -    my $query        = $operands[0];
> > +    my $query        = $operands[0] // "";
> >      my $simple_query = $operands[0];
> > 
> > And looking a bit later, I see:
> >     for ($query_cgi,$simple_query) {
> >         s/"//g;
> >     }
> > Shouldnt this trigger a warn too when undefined?
> 
> The code sets it undef later, and simple_query is not used between the two
> places

Hmm. Not important but I do not see that it is set to undef later, and if it is, then it will surely trigger the uninitialized warn that we want to resolve.
If it is not set, then it depends on $operands[0]
Comment 7 Jonathan Druart 2021-07-12 13:49:44 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 8 Jonathan Druart 2021-07-13 07:47:21 UTC
Created attachment 122794 [details] [review]
Bug 28483: Fix t/Search/buildQuery.t
Comment 9 Jonathan Druart 2021-07-13 07:52:21 UTC
Follow-up pushed to master.
Comment 10 Kyle M Hall 2021-07-16 13:48:39 UTC
Pushed to 21.05.x for 21.05.02
Comment 11 Fridolin Somers 2021-08-02 21:42:37 UTC
Depends on Bug 21260 not in 20.11.x