From 8fdfa80609600b58f3022f0dfa5a742e8f41b896 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Kyle M Hall Signed-off-by: Fridolin Somers --- 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