Bug 15339 - TestBuilder build parameter warnings
Summary: TestBuilder build parameter warnings
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Persson
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 19268
Blocks:
  Show dependency treegraph
 
Reported: 2015-12-09 12:05 UTC by Martin Persson
Modified: 2017-10-10 09:50 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15339: TestBuilder warnings (code) (1.29 KB, patch)
2015-12-09 12:27 UTC, Martin Persson
Details | Diff | Splinter Review
Bug 15339: TestBuilder warnings (tests) (1.48 KB, patch)
2015-12-09 12:27 UTC, Martin Persson
Details | Diff | Splinter Review
Bug 15339: TestBuilder warnings (code) (1.19 KB, patch)
2017-09-06 16:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15339: TestBuilder warnings (tests) (2.04 KB, patch)
2017-09-06 16:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15339: Improve parameter validation (1.41 KB, patch)
2017-09-06 16:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15339: TestBuilder warnings (code) (1.28 KB, patch)
2017-09-07 10:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15339: TestBuilder warnings (tests) (2.13 KB, patch)
2017-09-07 10:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15339: Improve parameter validation (1.50 KB, patch)
2017-09-07 10:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15339: [QA Follow-up] Add a warning too when source is not supplied (2.92 KB, patch)
2017-09-07 10:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15339: Remove extra 'my' (990 bytes, patch)
2017-09-07 16:55 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 Martin Persson 2015-12-09 12:05:09 UTC
TestBuilder's build function ignores any parameters that it does not recognize. If the developer forgets to wrap the column values in a 'value' hashref he or she might waste time checking the code rather than the tests. This patch makes TestBuilder issue a warning whenever unregonized parameters are passed to build.
Comment 1 Martin Persson 2015-12-09 12:27:03 UTC
Created attachment 45527 [details] [review]
Bug 15339: TestBuilder warnings (code)

Makes TestBuilder::build() alert the user when unreognized
parameters are passed, which happens when the user supplies
the column values directly, forgetting the 'value' hash.

This patch holds the code changes. Examples of the kind of
errors that it catches are in the tests (separate patch).

Sponsored-By: Halland County Library
Comment 2 Martin Persson 2015-12-09 12:27:20 UTC
Created attachment 45528 [details] [review]
Bug 15339: TestBuilder warnings (tests)

Makes TestBuilder::build() alert the user when unreognized parameters
are passed, which happens when the user supplies the column values
directly, forgetting the 'value' hash.

This patch contains the tests that doubles as a demonstration
of the kind of error the patch is intended to prevent.

Sponsored-By: Halland County Library
Comment 3 Martin Persson 2015-12-09 20:07:17 UTC
Realized there are some shortcomings that I need to fix before this is worth looking at.
Comment 4 Jonathan Druart 2017-09-06 16:43:10 UTC
Created attachment 66892 [details] [review]
Bug 15339: TestBuilder warnings (code)

Makes TestBuilder::build() alert the user when unreognized
parameters are passed, which happens when the user supplies
the column values directly, forgetting the 'value' hash.

This patch holds the code changes. Examples of the kind of
errors that it catches are in the tests (separate patch).

Sponsored-By: Halland County Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Jonathan Druart 2017-09-06 16:43:14 UTC
Created attachment 66893 [details] [review]
Bug 15339: TestBuilder warnings (tests)

Makes TestBuilder::build() alert the user when unreognized parameters
are passed, which happens when the user supplies the column values
directly, forgetting the 'value' hash.

This patch contains the tests that doubles as a demonstration
of the kind of error the patch is intended to prevent.

Sponsored-By: Halland County Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2017-09-06 16:43:18 UTC
Created attachment 66894 [details] [review]
Bug 15339: Improve parameter validation

Only value and source are allowed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2017-09-06 16:43:49 UTC
Patches have been rebased and adapt to the new code.
Comment 8 Marcel de Rooy 2017-09-07 10:09:58 UTC
Created attachment 66921 [details] [review]
Bug 15339: TestBuilder warnings (code)

Makes TestBuilder::build() alert the user when unreognized
parameters are passed, which happens when the user supplies
the column values directly, forgetting the 'value' hash.

This patch holds the code changes. Examples of the kind of
errors that it catches are in the tests (separate patch).

Sponsored-By: Halland County Library

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-09-07 10:10:06 UTC
Created attachment 66922 [details] [review]
Bug 15339: TestBuilder warnings (tests)

Makes TestBuilder::build() alert the user when unreognized parameters
are passed, which happens when the user supplies the column values
directly, forgetting the 'value' hash.

This patch contains the tests that doubles as a demonstration
of the kind of error the patch is intended to prevent.

Sponsored-By: Halland County Library

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-09-07 10:10:11 UTC
Created attachment 66923 [details] [review]
Bug 15339: Improve parameter validation

Only value and source are allowed

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-09-07 10:10:16 UTC
Created attachment 66924 [details] [review]
Bug 15339: [QA Follow-up] Add a warning too when source is not supplied

When you do not supply a source and add a few wrong parameters, you
would not be warned. Because build simply returns undef.
Adding a carp and a test for that situation too.

Note: In the earlier subtest 'trivial tests' build was called without
source. This now generates a warning. We just catch if there is a warning
and test the actual warning itself later on.

Test plan:
Run t/db_dependent/TestBuilder.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Jonathan Druart 2017-09-07 16:55:26 UTC
Created attachment 66951 [details] [review]
Bug 15339: Remove extra 'my'
Comment 13 Jonathan Druart 2017-09-07 18:06:40 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 14 Marcel de Rooy 2017-09-08 05:37:56 UTC
(In reply to Jonathan Druart from comment #12)
> Created attachment 66951 [details] [review] [review]
> Bug 15339: Remove extra 'my'

O my. Thx for catching that..
Comment 15 Fridolin SOMERS 2017-10-10 09:50:59 UTC
Enhancement not pushed to 17.05.x