Locate actual wheels in WHEELS_DIR

- The wheels need to be stored in a way that they can be modified easily rather than pool which is volatile. - By default the pool is used as always. - When cloning the path is explicitly logged. Fixes: https://progress.opensuse.org/issues/134390
os-autoinst · Sep 20, 2023 · b3ebf1e · b3ebf1e
1 parent d8ad077
commit b3ebf1e
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 37 deletions.
diff --git a/OpenQA/Isotovideo/Runner.pm b/OpenQA/Isotovideo/Runner.pm
@@ -214,8 +214,7 @@ sub checkout_code ($self) {
 
     $bmwqemu::vars{TEST_GIT_HASH} = checkout_git_refspec($bmwqemu::vars{CASEDIR} => 'TEST_GIT_REFSPEC');
 
-    $bmwqemu::vars{WHEELS_DIR} ||= $bmwqemu::vars{CASEDIR};
-    checkout_wheels($bmwqemu::vars{WHEELS_DIR});
+    checkout_wheels($bmwqemu::vars{CASEDIR}, $bmwqemu::vars{WHEELS_DIR});
 }
 
 sub _flush_std ($) {

diff --git a/OpenQA/Isotovideo/Utils.pm b/OpenQA/Isotovideo/Utils.pm
@@ -7,6 +7,7 @@ use Mojo::Base -base, -signatures;
 use Mojo::URL;
 use Mojo::File qw(path);
 use JSON::Validator;
+use Cwd 'cwd';
 use YAML::XS;    # Required by JSON::Validator as a runtime dependency
 use YAML::PP;
 
@@ -42,7 +43,7 @@ sub clone_git ($local_path, $clone_url, $clone_depth, $branch, $dir, $dir_variab
         bmwqemu::diag "Skipping to clone '$clone_url'; $local_path already exists";
         return 1;
     }
-    bmwqemu::fctinfo "Cloning git URL '$clone_url'";
+    bmwqemu::fctinfo "Cloning git URL '$clone_url' into '" . cwd . "'";
     my $branch_args = '';
     if ($branch) {
         bmwqemu::fctinfo "Checking out git refspec/branch '$branch'";
@@ -138,8 +139,8 @@ optional git refspec to checkout.
 If no wheels are configured the function returns early.
 
 =cut
-sub checkout_wheels ($dir) {
-    my $specfile = path($dir, 'wheels.yaml');
+sub checkout_wheels ($case_dir, $wheels_dir = undef) {
+    my $specfile = path($case_dir, 'wheels.yaml');
     return 1 unless -e $specfile;
 
     my $schema_file = "$bmwqemu::scriptdir/schema/Wheels-01.yaml";
@@ -150,15 +151,18 @@ sub checkout_wheels ($dir) {
     $validator->schema($schema_file);
     my $spec = YAML::PP->new->load_file($specfile);
     my @errors = $validator->validate($spec);
-    die 'Invalid YAML: ' . join(',', @errors) if @errors;
-    die 'Unsupported version: Version must be "v0.1"' if $spec->{version} ne 'v0.1';
+    die "Invalid YAML ($specfile): " . join(',', @errors) if @errors;
+    die "Unsupported version ($specfile): Version must be 'v0.1'" if $spec->{version} ne 'v0.1';
 
+    my $old_cwd = cwd;
+    chdir $wheels_dir if defined $wheels_dir;
     foreach my $repo (@{$spec->{wheels}}) {
         $repo = "https://github.com/$repo.git" unless $repo =~ qr/^http/;
         if (my $clone = checkout_git_repo_and_branch($specfile, repo => $repo)) {
             unshift @INC, "$clone/lib";
         }
     }
+    chdir $old_cwd;
     return 0;
 }
 

diff --git a/autotest.pm b/autotest.pm
@@ -19,6 +19,7 @@ use Scalar::Util 'blessed';
 use Mojo::IOLoop::ReadWriteProcess 'process';
 use Mojo::IOLoop::ReadWriteProcess::Session 'session';
 use Mojo::File qw(path);
+use File::Glob qw(bsd_glob);
 
 our @EXPORT_OK = qw(loadtest $selected_console $last_milestone_console query_isotovideo);
 
@@ -34,8 +35,9 @@ script called main.pm. This is either located in $vars{PRODUCTDIR} or
 $vars{CASEDIR} (e.g. <distribution>/products/<product>/main.pm).
 
 Wheels can be used to add functionality from other repositories. If a file
-wheels.yaml is present the specified git repositories are cloned before tests
-are run. $vars{WHEELS_DIR} defaults to $vars{CASEDIR} if not set explicitly.
+`wheels.yaml` is present the specified git repositories are cloned before tests
+are run. $vars{WHEELS_DIR} defaults to the working directory of `isotovideo`
+if not set explicitly and determines where wheels are stored.
 
 This script does not actually run the tests, but queues them to be run by
 autotest.pm. A test is queued by calling the loadtest function which is also
@@ -45,7 +47,8 @@ loadtest is called.
 =cut
 
 sub find_script ($script) {
-    if (defined(my $wheel = (glob Cwd::getcwd . "/*/tests/$script"))) {
+    my $wheels_dir = $bmwqemu::vars{WHEELS_DIR} // Cwd::getcwd;
+    if (defined(my $wheel = bsd_glob "$wheels_dir/*/tests/$script")) {
         return $wheel;
     }
     my $casedir = $bmwqemu::vars{CASEDIR};

diff --git a/doc/backend_vars.asciidoc b/doc/backend_vars.asciidoc
@@ -9,7 +9,7 @@ Variable;Values allowed;Default value;Explanation
 CASEDIR;string;;Absolute or relative path to the test distribution. Specifying the variable is mandatory. Can also be a Git repository URL of a test distribution to checkout with an optional refspec or Git commit hash into the current directory. It tries to follow the definition of https://docs.npmjs.com/cli/v7/configuring-npm/package-json#git-urls-as-dependencies (e.g. `https://github.com/os-autoinst/os-autoinst-distri-opensuse.git#feature/test` or `git@github.com:os-autoinst/os-autoinst-distri-opensuse.git`). Note that cloning via SSH is only possible if Git is configured accordingly on the host where isotovideo is executed. Can be combined with `TEST_GIT_REFSPEC` and `NEEDLES_GIT_REFSPEC`. Relative paths are considered relative to the working directory of `isotovideo`.
 PRODUCTDIR;string;;Absolute or relative path to the optional "product directory" which includes the test schedule entry point `main.pm` as well as a `needles` subdirectory with the needles to load. Defaults to `CASEDIR`. Relative paths are considered relative to `CASEDIR`.
 NEEDLES_DIR;string;;Absolute or relative path to the needles subdirectory. Defaults to `needles` within `PRODUCTDIR`. Can also be a Git repository URL, comparable to `CASEDIR`. Relative paths are considered relative to the working directory of `isotovideo`.
-WHEELS_DIR;string;;Absolute or relative path to the wheels subdirectory. Defaults to `CASEDIR`. Relative paths are considered relative to the working directory of `isotovideo`.
+WHEELS_DIR;string;;Absolute or relative path to the directory wheels are stored in if `CASEDIR` contains a `wheels.yaml`. Defaults to the working directory of `isotovideo`. Relative paths are considered relative to the working directory of `isotovideo`.
 INCLUDE_MODULES;string;;comma separated names or fullnames of test modules to be included while excluding all that do not match, e.g. "boot,mod1"
 EXCLUDE_MODULES;string;;comma separated names or fullnames of test modules to exclude. Can be combined with INCLUDE_MODULES and has precedence, e.g. to additionally exclude modules based on an include-list
 SCHEDULE;string;;comma or space separated list of relative paths to test modules within CASEDIR to be scheduled instead of evaluating a schedule from the test distributions main.pm file, e.g. "boot,console/mod1". Test modules can be specified without the implicit file extension '.pm'.

diff --git a/t/14-isotovideo.t b/t/14-isotovideo.t
@@ -17,7 +17,7 @@ use Cwd 'abs_path';
 use Mojo::File qw(tempdir path);
 use Mojo::JSON qw(decode_json);
 use Mojo::Util qw(scope_guard);
-use OpenQA::Isotovideo::Utils qw(checkout_wheels handle_generated_assets);
+use OpenQA::Isotovideo::Utils qw(checkout_wheels handle_generated_assets load_test_schedule);
 use OpenQA::Isotovideo::CommandHandler;
 
 my $dir = tempdir("/tmp/$FindBin::Script-XXXX");
@@ -128,49 +128,83 @@ subtest 'isotovideo with git refspec specified' => sub {
 subtest 'isotovideo with wheels' => sub {
     chdir($pool_dir);
     unlink('vars.json') if -e 'vars.json';
-
     $bmwqemu::scriptdir = "$Bin/..";
-    my $wheels_dir = "$data_dir";
-    my $specfile = path("$wheels_dir/wheels.yaml");
+    my $case_dir = "$data_dir/tests";
+    my $wheels_dir = "$data_dir/wheels_dir";
+    my $specfile = path($case_dir)->make_path->child('wheels.yaml');
     $specfile->spew("wheels: [foo/bar]");
     throws_ok { checkout_wheels(
-            "$wheels_dir") } qr@Invalid.*Missing property@, 'invalid YAML causes error';
+            $case_dir, $wheels_dir) } qr@Invalid.*\Q$specfile\E.*Missing property@, 'invalid YAML causes error';
     $specfile->spew("version: v99\nwheels: [foo/bar]");
     throws_ok { checkout_wheels(
-            "$wheels_dir") } qr@Unsupported version@, 'unsupported version';
+            $case_dir, $wheels_dir) } qr@Unsupported version.*\Q$specfile\E.*@, 'unsupported version';
     $specfile->spew("version: v0.1\nwheels: [https://github.com/foo/bar.git]");
     my $utils_mock = Test::MockModule->new('OpenQA::Isotovideo::Utils');
+    my $bmwqemu_mock = Test::MockModule->new('bmwqemu');
     my @repos;
-    $utils_mock->redefine(checkout_git_repo_and_branch => sub ($dir_variable, %args) { push @repos, $args{repo} });
-    checkout_wheels("$wheels_dir");
-    is($repos[0], 'https://github.com/foo/bar.git', 'repo with full URL');
+    $utils_mock->redefine(clone_git => sub ($local_path, $clone_url, $clone_depth, $branch, $dir, $dir_variable, $direct_fetch) {
+            push @repos, [$clone_url, $branch];
+            return 1;
+    });
+    checkout_wheels($case_dir, $wheels_dir);
+    is($repos[0][0], 'https://github.com/foo/bar.git', 'repo with full URL');
     is(scalar @repos, 1, 'one wheel');
     $specfile->spew("version: v0.1\nwheels: [https://github.com/foo/bar.git#branch]");
-    checkout_wheels("$wheels_dir");
-    is($repos[1], 'https://github.com/foo/bar.git#branch', 'repo URL with branch');
+    checkout_wheels($case_dir, $wheels_dir);
+    is($repos[1][0], 'https://github.com/foo/bar.git', 'repo URL with branch');
+    is($repos[1][1], 'branch', 'repo URL with branch');
     is(scalar @repos, 2, 'one wheel');
     $specfile->spew("version: v0.1\nwheels: [foo/bar]");
-    checkout_wheels("$wheels_dir");
+    checkout_wheels($case_dir, $wheels_dir);
     is(scalar @repos, 3, 'one wheel');
-    is($repos[2], 'https://github.com/foo/bar.git', 'only wheel');
+    is($repos[2][0], 'https://github.com/foo/bar.git', 'only wheel');
     $specfile->spew("version: v0.1\nwheels: [foo/bar, spam/eggs]");
-    checkout_wheels("$wheels_dir");
-    is($repos[4], 'https://github.com/spam/eggs.git', 'second wheel');
+    checkout_wheels($case_dir, $wheels_dir);
+    is($repos[4][0], 'https://github.com/spam/eggs.git', 'second wheel');
     is(scalar @repos, 5, 'two wheels');
     $specfile->remove;
-    is(checkout_wheels("$wheels_dir"), 1, 'no wheels');
+    is(checkout_wheels($case_dir, $wheels_dir), 1, 'no wheels');
     is(scalar @repos, 5, 'git never called');
 
     # also verify that isotovideo invokes the wheel code correctly
-    $bmwqemu::vars{CASEDIR} = "$data_dir/tests";
     $specfile->spew("version: v0.1\nwheels: [copy/writer]");
-    path($pool_dir, 'writer', 'lib', 'Copy', 'Writer')->make_path->child('Content.pm')->spew("package Copy::Writer::Content; use Mojo::Base 'Exporter'; our \@EXPORT_OK = qw(write); sub write {}; 1");
-    path($pool_dir, 'writer', 'tests', 'pen')->make_path->child('ink.pm')->spew("use Mojo::Base 'basetest'; use Copy::Writer::Content 'write'; sub run {}; 1");
-    my $log = combined_from { isotovideo(
-            opts => "wheels_dir=$wheels_dir casedir=$data_dir/tests schedule=pen/ink _exit_after_schedule=1") };
-    like $log, qr@Skipping to clone.+copy/writer@, 'already cloned wheel picked up';
-    like $log, qr/scheduling ink/, 'module from the wheel scheduled';
-    rmtree "$pool_dir/writer";
+
+    my @warnings;
+    my @infos;
+    my @diags;
+    $bmwqemu_mock->redefine(fctwarn => sub { push @warnings, $_[0] });
+    $bmwqemu_mock->redefine(fctinfo => sub { push @infos, $_[0] });
+    $bmwqemu_mock->redefine(diag => sub { push @diags, $_[0] });
+
+    my @tests = ("$data_dir/wheels_dir", undef);
+    for my $i (0 .. $#tests) {
+        my $dir = $tests[$i];
+        chdir $pool_dir;
+        $wheels_dir = $dir // $pool_dir;
+        $bmwqemu::vars{CASEDIR} = $case_dir;
+        $bmwqemu::vars{PRODUCTDIR} = $pool_dir;
+        $bmwqemu::vars{SCHEDULE} = "pen/ink$i";
+        $bmwqemu::vars{WHEELS_DIR} = $dir;
+
+        subtest "wheels in $wheels_dir" => sub {
+            @diags = ();
+            path($wheels_dir, 'writer', 'lib', 'Copy', 'Writer')->make_path->child("Content$i.pm")->spew(<<~"EOM");
+            package Copy::Writer::Content$i;
+            use Mojo::Base 'Exporter';
+            our \@EXPORT_OK = qw(write);
+            sub write { "val$i"};
+            1;
+            EOM
+            path($wheels_dir, 'writer', 'tests', 'pen')->make_path->child("ink$i.pm")->spew("use Mojo::Base 'basetest'; use Copy::Writer::Content$i 'write'; sub run {}; 1");
+            is(0, checkout_wheels($case_dir, $wheels_dir), 'wheels checkout out');
+            load_test_schedule;
+            like $diags[0], qr/scheduling ink$i/, 'module from the wheel scheduled';
+            my $ret = eval "Copy::Writer::Content${i}::write()";
+            is $ret, "val$i", 'called function in wheel library returns expected value';
+            rmtree "$wheels_dir/writer";
+        };
+    }
+    $specfile->remove;
 };
 
 subtest 'productdir variable relative/absolute' => sub {
@@ -310,5 +344,7 @@ done_testing();
 
 END {
     rmtree "$Bin/data/tests/product";
-    unlink("$data_dir/wheels.yaml") if -e "$data_dir/wheels.yaml";
+    rmtree "$data_dir/wheels_dir/writer";
+    rmtree "$pool_dir/writer";
+    unlink("$data_dir/tests/wheels.yaml") if -e "$data_dir/tests/wheels.yaml";
 }
diff --git a/tools/invoke-tests b/tools/invoke-tests
@@ -44,7 +44,7 @@ db="$build_directory/cover_db"
 # set Perl module include path and coverage options
 export PERL5LIB="$source_directory:$source_directory/ppmclibs/blib/lib:$source_directory/ppmclibs/blib/arch/auto/tinycv:$PERL5LIB"
 if [[ $WITH_COVER_OPTIONS ]]; then
-    ignore="external/|tools/|t/data/tests/tests/|/tmp|/CheckGitStatus.pm|$prove_path"
+    ignore="external/|tools/|t/data/tests/tests/|t/data/wheels_dir|/tmp|/CheckGitStatus.pm|$prove_path"
     # add ' -MOpenQA::Test::PatchDeparse' for older OS versions to avoid warnings
     export PERL5OPT="-I$source_directory/t/lib -I$source_directory/external/os-autoinst-common/lib -MOpenQA::Test::CheckGitStatus -MDevel::Cover=-db,$db,-ignore,$ignore,-coverage,statement"
 fi