Skip to content

Commit

Permalink
Move 'dsn' functionality into only place where used
Browse files Browse the repository at this point in the history
In b1da48e I removed the "dsn" function and its usage as it looked
like that would not be necessary anymore at all and we did not have any
test coverage for that. This turned out to be a false assumption leading
to a production openQA instance to not start anymore with the error
message

```
openqa-webui-daemon: Can't connect to data source 'HASH(0x55d82c060598)' because I can't work out what driver to use (it doesn't seem to contain a 'dbi:driver:' prefix and the DBI_DRIVER env var is not set)>
```

so the change was quickly reverted.

This commit now moves the 'dsn' functionality to the only place where
it is needed, simplifies the call and adds complete test coverage for
the according method with a separate, new test module
"t/15-shared-plugin-gru.t".

Tests did not cover the 'dsn' function anyway.

Verified by calling

```
env OPENQA_BASEDIR=$PWD/t/data OPENQA_DATABASE=test script/openqa-gru
```

locally.
  • Loading branch information
okurz committed Aug 2, 2024
1 parent 5874bef commit ca39b9d
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 16 deletions.
5 changes: 0 additions & 5 deletions lib/OpenQA/Schema.pm
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ sub disconnect_db () {
$SINGLETON = undef;
}

sub dsn {
my $self = shift;
return $self->storage->connect_info->[0]->{dsn};
}

sub deploy ($self, $force_overwrite = 0) {
# lock config file to ensure only one thing will deploy/upgrade DB at once
# we use a file in prjdir/db as the lock file as the install process and
Expand Down
30 changes: 19 additions & 11 deletions lib/OpenQA/Shared/Plugin/Gru.pm
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,31 @@ sub register_tasks ($self) {
);
}

# allow the continuously polled stats to be available on an
# unauthenticated route to prevent recurring broken requests to the login
# provider if not logged in
#
# hard/impossible to mock due to name collision of "remove" method on
# Test::MockObject, hence marking as
# uncoverable statement
sub _allow_unauthenticated_minion_stats ($app) {
my $route = $app->routes->find('minion_stats')->remove; # uncoverable statement
$app->routes->any('/minion')->add_child($route); # uncoverable statement
}

sub register ($self, $app, $config) {
$self->app($app) unless $self->app;
my $schema = $app->schema;

my $conn = Mojo::Pg->new;
if (ref $schema->storage->connect_info->[0] eq 'HASH') {
$self->dsn($schema->dsn);
$conn->username($schema->storage->connect_info->[0]->{user});
$conn->password($schema->storage->connect_info->[0]->{password});
my $connect_info = $schema->storage->connect_info->[0];
if (ref $connect_info eq 'HASH') {
$self->dsn($connect_info->{dsn});
$conn->username($connect_info->{user});
$conn->password($connect_info->{password});
}
else {
$self->dsn($schema->storage->connect_info->[0]);
$self->dsn($connect_info);
}
$conn->dsn($self->dsn());

Expand Down Expand Up @@ -82,12 +95,7 @@ sub register ($self, $app, $config) {
# Enable the Minion Admin interface under /minion
my $auth = $app->routes->under('/minion')->to('session#ensure_admin');
$app->plugin('Minion::Admin' => {route => $auth});
# allow the continuously polled stats to be available on an
# unauthenticated route to prevent recurring broken requests to the login
# provider if not logged in
my $route = $app->routes->find('minion_stats')->remove;
$app->routes->any('/minion')->add_child($route);

_allow_unauthenticated_minion_stats($app);
my $gru = OpenQA::Shared::Plugin::Gru->new($app);
$app->helper(gru => sub ($c) { $gru });
}
Expand Down
41 changes: 41 additions & 0 deletions t/15-shared-plugin-gru.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/usr/bin/env perl
# Copyright SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

use Test::Most;

use Test::Warnings ':report_warnings';
use Test::MockModule;
use Test::MockObject;
use FindBin;
use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib";
use OpenQA::Test::TimeLimit '5';
use OpenQA::App;
use OpenQA::Shared::Plugin::Gru; # SUT

ok my $gru = OpenQA::Shared::Plugin::Gru->new, 'can instantiate gru plugin';

my $connect_info = {dsn => 'my_dsn', user => 'user', password => 'foo'};
my $storage = Test::MockObject->new->set_always(connect_info => [$connect_info]);
my $schema = Test::MockObject->new->set_always(storage => $storage)->set_true('search_path_for_tests');
my %config = (misc_limits => {minion_job_max_age => 0});
my $minion = Test::MockObject->new->set_true('on');
my $log = Test::MockObject->new->set_true('level', 'info');
my $under = Test::MockObject->new->set_true('to');
my $routes = Test::MockObject->new->set_always(under => $under);
my $app
= Test::MockObject->new->set_always(schema => $schema)->set_always(config => \%config)->set_always(minion => $minion)
->set_always(log => $log)->set_always(routes => $routes)->set_true('plugin', 'helper');
OpenQA::App->set_singleton($app);
my $mock = Test::MockModule->new('OpenQA::Shared::Plugin::Gru');
$mock->noop('_allow_unauthenticated_minion_stats');
my $pg = Test::MockObject->new->set_true('dsn', 'username', 'password', 'search_path', 'find');
my $pg_module = Test::MockModule->new('Mojo::Pg')->redefine(new => $pg);
ok $gru->register($app, undef), 'can register gru plugin';
$pg->called_ok('username', 'pg connection initialized with username');
is(($pg->call_args(0))[1], $connect_info->{user}, 'pg connection username is correct');
is(($pg->call_args(2))[1], $connect_info->{password}, 'pg connection password is correct');
is(($app->call_args(4))[1], 'Minion', 'minion initialized');
is(($app->call_args(4))[2]->{Pg}, $pg, 'pg connection initialized on minion');

done_testing;

0 comments on commit ca39b9d

Please sign in to comment.