Bug 17426 - AutoCommit should not be set in tests
Summary: AutoCommit should not be set in tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-11 12:15 UTC by Jonathan Druart
Modified: 2017-06-14 22:13 UTC (History)
5 users (show)

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


Attachments
Bug 17426: Do not set AutoCommit in tests (2.60 KB, patch)
2016-10-11 12:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17426: Do not set AutoCommit in tests (2.64 KB, patch)
2016-10-11 13:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17426: Do not set AutoCommit in tests (2.74 KB, patch)
2016-10-20 11:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-10-11 12:15:20 UTC
git grep AutoCommit t | grep 1

Should not return any occurrences
Comment 1 Jonathan Druart 2016-10-11 12:16:53 UTC
Created attachment 56171 [details] [review]
Bug 17426: Do not set AutoCommit in tests

git grep AutoCommit t | grep 1

highlights that 1 test use AutoCommit set, it should not

The other change removes the FIXME should has been fixed.

Test plan:
  prove t/db_dependent/BackgroundJob.t
  t/db_dependent/Patron/Borrower_Discharge.t

should still return green
Comment 2 Marc Véron 2016-10-11 13:46:11 UTC
Created attachment 56192 [details] [review]
Bug 17426: Do not set AutoCommit in tests

  git grep AutoCommit t | grep 1

highlights that 1 test use AutoCommit set, it should not

The other change removes the FIXME should has been fixed.

Test plan:
  prove t/db_dependent/BackgroundJob.t
  t/db_dependent/Patron/Borrower_Discharge.t

should still return green

Signed-off-by: Marc <veron@veron.ch>
Comment 3 Marcel de Rooy 2016-10-20 11:22:49 UTC
Created attachment 56692 [details] [review]
Bug 17426: Do not set AutoCommit in tests

git grep AutoCommit t | grep 1

highlights that 1 test use AutoCommit set, it should not

The other change removes the FIXME should has been fixed.

Test plan:
  prove t/db_dependent/BackgroundJob.t
  t/db_dependent/Patron/Borrower_Discharge.t

should still return green

Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2016-10-20 11:25:39 UTC
+# FIXME Should be a Koha::Object object
+is( ref(Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} })), 'Koha::Schema::Result::Discharge', 'Discharge request sent' );

Did you open a new bug report for that?
Comment 5 Jonathan Druart 2016-10-20 11:36:55 UTC
(In reply to Marcel de Rooy from comment #4)
> +# FIXME Should be a Koha::Object object
> +is( ref(Koha::Patron::Discharge::request({ borrowernumber =>
> $patron->{borrowernumber} })), 'Koha::Schema::Result::Discharge', 'Discharge
> request sent' );
> 
> Did you open a new bug report for that?

Nope, it's more a note than a FIXME. The whole Koha::Patron::Discharge would need a rewrite not to use the DBIx::Class rs directly.
Comment 6 Kyle M Hall 2016-10-21 14:34:30 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 7 Frédéric Demians 2016-10-22 13:02:16 UTC
Pushed in 16.05. Will be in 16.05.05.
Comment 8 Julian Maurice 2016-10-24 10:47:18 UTC
Pushed in 3.22.x, will be in 3.22.12