Bug 17426

Summary: AutoCommit should not be set in tests
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: f.demians, julian.maurice, kyle, m.de.rooy, veron
Version: unspecified   
Hardware: All   
OS: All   
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
Bug 17426: Do not set AutoCommit in tests
Bug 17426: Do not set AutoCommit in tests

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