Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Avoid syncing tests via the cache service when using Git anyway" #5498

Merged
merged 1 commit into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 1 addition & 6 deletions lib/OpenQA/Worker/Engines/isotovideo.pm
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,6 @@ sub sync_tests ($cache_client, $job, $vars, $shared_cache, $rsync_source, $remai
});
}

sub _is_job_only_relying_on_git ($vars) {
my ($cd, $nd) = ($vars->{CASEDIR}, $vars->{NEEDLES_DIR});
return defined($cd) && looks_like_url_with_scheme($cd) && (!defined($nd) || looks_like_url_with_scheme($nd));
}

sub do_asset_caching ($job, $vars, $cache_dir, $assetkeys, $webui_host, $pooldir, $callback) {
my $cache_client = OpenQA::CacheService::Client->new;
cache_assets(
Expand All @@ -270,7 +265,7 @@ sub do_asset_caching ($job, $vars, $cache_dir, $assetkeys, $webui_host, $pooldir
sub ($error) {
return $callback->($error) if $error;
my $rsync_source = $job->client->testpool_server;
return $callback->(undef) if !$rsync_source || _is_job_only_relying_on_git($vars);
return $callback->(undef) unless $rsync_source;
my $attempts = CACHE_SERVICE_TEST_SYNC_ATTEMPTS;
my $shared_cache = catdir($cache_dir, base_host($webui_host));
sync_tests($cache_client, $job, $vars, $shared_cache, $rsync_source, $attempts, $callback);
Expand Down
34 changes: 2 additions & 32 deletions t/24-worker-engine.t
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use OpenQA::Utils qw(testcasedir productdir needledir locate_asset base_host);
use Mojo::Base -base;
has id => 42;
has worker => undef;
has client => sub { Test::FakeClient->new };
sub post_setup_status { 1 }
sub is_stopped_or_stopping { 0 }
}
Expand All @@ -53,7 +52,6 @@ use OpenQA::Utils qw(testcasedir productdir needledir locate_asset base_host);
has worker_id => 1;
has webui_host => 'localhost';
has service_port_delta => 2;
has testpool_server => 'fake-testpool-server';
}

$ENV{OPENQA_CONFIG} = "$FindBin::Bin/data/24-worker-overall";
Expand Down Expand Up @@ -198,7 +196,7 @@ sub _mock_cache_service_client ($status_data, $error = undef) {
return $cache_client_mock;
}

subtest 'problems and special cases when caching assets' => sub {
subtest 'problems when caching assets' => sub {
my %fake_status = (status => 'processed', output => 'Download of "FOO" failed: 404 Not Found');
my $cache_client_mock = _mock_cache_service_client \%fake_status;
$cache_client_mock->redefine(asset_path => 'some/path');
Expand All @@ -207,9 +205,8 @@ subtest 'problems and special cases when caching assets' => sub {
my $error = 'not called';
my $cb = sub ($err) { $error = $err; Mojo::IOLoop->stop };
my $job = Test::FakeJob->new;
my %vars = (ISO_1 => 'FOO', CASEDIR => 'https://foo.git', NEEDLES_DIR => 'https://bar.git');
my @assets = ('ISO_1');
my @args = ($job, \%vars, \@assets, {ISO_1 => 'iso'}, 'webuihost', undef, $cb);
my @args = ($job, {ISO_1 => 'FOO'}, \@assets, {ISO_1 => 'iso'}, 'webuihost', undef, $cb);
OpenQA::Worker::Engines::isotovideo::cache_assets(OpenQA::CacheService::Client->new, @args);
Mojo::IOLoop->start;
is $error->{error}, 'Failed to send asset request for FOO: some enqueue error',
Expand All @@ -230,33 +227,6 @@ subtest 'problems and special cases when caching assets' => sub {
OpenQA::Worker::Engines::isotovideo::cache_assets(OpenQA::CacheService::Client->new, @args);
Mojo::IOLoop->start;
is $error->{error}, 'some severe error', 'job not "processed" due to some error';

subtest 'test sync skipped for Git-only jobs' => sub {
my $isotovideo_mock = Test::MockModule->new('OpenQA::Worker::Engines::isotovideo');
my $sync_tests_called;
$isotovideo_mock->redefine(cache_assets => sub (@args) { $args[-1]->(undef) });
$isotovideo_mock->redefine(sync_tests => sub (@) { $sync_tests_called = 1 });

OpenQA::Worker::Engines::isotovideo::do_asset_caching(@args);
is $error, undef, 'callback called without error (1)';
is $sync_tests_called, undef, 'sync tests was not called for Git-only job (separate needles repo)';

delete $vars{NEEDLES_DIR}; # no NEEDLES_DIR means needles are within CASEDIR
OpenQA::Worker::Engines::isotovideo::do_asset_caching(@args);
is $error, undef, 'callback called without error (2)';
is $sync_tests_called, undef, 'sync tests was not called for Git-only job (combined repo)';

$vars{NEEDLES_DIR} = 'relative/path'; # considered relative to default needles, relying on sync
OpenQA::Worker::Engines::isotovideo::do_asset_caching(@args);
is $error, undef, 'callback called without error (3)';
is $sync_tests_called, 1, 'sync tests called with relative NEEDLES_DIR (relative to default needles)';

delete $vars{CASEDIR}; # no CASEDIR means using default checkout, relying on sync
undef $sync_tests_called;
OpenQA::Worker::Engines::isotovideo::do_asset_caching(@args);
is $error, undef, 'callback called without error (4)';
is $sync_tests_called, 1, 'sync tests was called for job not using Git at all';
};
};

subtest '_handle_asset_processed' => sub {
Expand Down