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.
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
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
Realized there are some shortcomings that I need to fix before this is worth looking at.
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>
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>
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>
Patches have been rebased and adapt to the new code.
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>
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>
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>
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>
Created attachment 66951 [details] [review] Bug 15339: Remove extra 'my'
Pushed to master for 17.11, thanks to everybody involved!
(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..
Enhancement not pushed to 17.05.x