From 8fdfa80609600b58f3022f0dfa5a742e8f41b896 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 3 Mar 2022 10:24:11 -0300
Subject: [PATCH] Bug 30181: (QA follow-up) Update BatchTest

This patch updates t::lib::Koha::BackgroundJob::BatchTest to the new
style, and also removes a couple stray cases in which job_id was still
passed as a parameter.

Tests are rewritten a bit, so they actually test more of the behaviors.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/BackgroundJobs.t
=> SUCCESS: Tests pass!
3. Sign off :-D

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

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
---
 t/db_dependent/Koha/BackgroundJobs.t  | 17 +++++++++++------
 t/lib/Koha/BackgroundJob/BatchTest.pm | 26 ++++++++++++--------------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/t/db_dependent/Koha/BackgroundJobs.t b/t/db_dependent/Koha/BackgroundJobs.t
index 476bac9a38..df2172f00c 100755
--- a/t/db_dependent/Koha/BackgroundJobs.t
+++ b/t/db_dependent/Koha/BackgroundJobs.t
@@ -41,8 +41,12 @@ my $net_stomp = Test::MockModule->new('Net::Stomp');
 $net_stomp->mock( 'send_with_receipt', sub { return 1 } );
 
 my $background_job_module = Test::MockModule->new('Koha::BackgroundJob');
-$background_job_module->mock( '_derived_class',
-    sub { t::lib::Koha::BackgroundJob::BatchTest->new } );
+$background_job_module->mock(
+    'type_to_class_mapping',
+    sub {
+        return { batch_test => 't::lib::Koha::BackgroundJob::BatchTest' };
+    }
+);
 
 my $data     = { a => 'aaa', b => 'bbb' };
 my $job_size = 10;
@@ -64,18 +68,19 @@ is( $new_job->size,   $job_size,    'job size retrieved correctly' );
 is( $new_job->status, "new",        'job has not started yet, status is new' );
 is( $new_job->type,   "batch_test", 'job type retrieved from ->job_type' );
 
+# FIXME: This behavior doesn't seem correct. It shouldn't be the background job's
+#        responsibility to return 'undef'. Some higher-level check should raise a
+#        proper exception.
 # Test cancelled job
 $new_job->status('cancelled')->store;
-my $processed_job =
-  t::lib::Koha::BackgroundJob::BatchTest->process( { job_id => $new_job->id } );
+my $processed_job = $new_job->process;
 is( $processed_job, undef );
 $new_job->discard_changes;
 is( $new_job->status, "cancelled", "A cancelled job has not been processed" );
 
 # Test new job to process
 $new_job->status('new')->store;
-$new_job =
-  t::lib::Koha::BackgroundJob::BatchTest->process( { job_id => $new_job->id } );
+$new_job = $new_job->process;
 is( $new_job->status,             "finished", 'job is new finished!' );
 is( scalar( @{ $new_job->messages } ), 10,    '10 messages generated' );
 is_deeply(
diff --git a/t/lib/Koha/BackgroundJob/BatchTest.pm b/t/lib/Koha/BackgroundJob/BatchTest.pm
index 2b6490dbfc..5c0edd3ffb 100644
--- a/t/lib/Koha/BackgroundJob/BatchTest.pm
+++ b/t/lib/Koha/BackgroundJob/BatchTest.pm
@@ -30,43 +30,41 @@ sub job_type {
 sub process {
     my ( $self, $args ) = @_;
 
-    my $job = Koha::BackgroundJobs->find( $args->{job_id} );
-
-    if ( !exists $args->{job_id} || !$job || $job->status eq 'cancelled' ) {
-        return;
-    }
+    # FIXME: This should happen when $self->SUPER::process is called instead
+    return
+      unless $self->status ne 'cancelled';
 
     my $job_progress = 0;
-    $job->started_on(dt_from_string)
+    $self->started_on(dt_from_string)
         ->progress($job_progress)
         ->status('started')
         ->store;
 
     my $report = {
-        total_records => $job->size,
+        total_records => $self->size,
         total_success => 0,
     };
     my @messages;
-    for my $i ( 0 .. $job->size - 1 ) {
+    for my $i ( 0 .. $self->size - 1 ) {
 
-        last if $job->get_from_storage->status eq 'cancelled';
+        last if $self->get_from_storage->status eq 'cancelled';
 
         push @messages, {
             type => 'success',
             i => $i,
         };
         $report->{total_success}++;
-        $job->progress( ++$job_progress )->store;
+        $self->progress( ++$job_progress )->store;
     }
 
-    my $job_data = decode_json $job->data;
+    my $job_data = decode_json $self->data;
     $job_data->{messages} = \@messages;
     $job_data->{report} = $report;
 
-    $job->ended_on(dt_from_string)
+    $self->ended_on(dt_from_string)
         ->data(encode_json $job_data);
-    $job->status('finished') if $job->status ne 'cancelled';
-    $job->store;
+    $self->status('finished') if $self->status ne 'cancelled';
+    $self->store;
 }
 
 sub enqueue {
-- 
2.39.5