Skip to content

Commit

Permalink
Merge pull request #1323 from okurz/fix/needle
Browse files Browse the repository at this point in the history
Fix undefined needle dir in dynamic needle downloader with proper variable names
  • Loading branch information
okurz committed Dec 13, 2019
2 parents ecd0ee7 + 05ae233 commit d867943
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 33 deletions.
4 changes: 2 additions & 2 deletions OpenQA/Isotovideo/NeedleDownloader.pm
Expand Up @@ -72,8 +72,8 @@ sub _add_download {

my $needle_name = $needle->{name};
my $latest_update = $needle->{t_updated};
my $needle_dir = $needle::needles_dir;
my $download_target = "$needle_dir/$needle_name.$extension";
my $needles_dir = $needle::needles_dir;
my $download_target = "$needles_dir/$needle_name.$extension";

if (my $target_stat = stat($download_target)) {
if (my $target_last_modified = $target_stat->[9] // $target_stat->[8]) {
Expand Down
28 changes: 14 additions & 14 deletions needle.pm
Expand Up @@ -33,7 +33,7 @@ use OpenQA::Benchmark::Stopwatch;

our %needles;
our %tags;
our $needledir;
our $needles_dir;
our $cleanuphandler;

sub is_click_point_valid {
Expand All @@ -47,12 +47,12 @@ sub is_click_point_valid {
sub new {
my ($classname, $jsonfile) = @_;

die 'needles not initialized via needle::init() before needle constructor called' unless defined $needledir;
die 'needles not initialized via needle::init() before needle constructor called' unless defined $needles_dir;

my $json;
if (ref $jsonfile eq 'HASH') {
$json = $jsonfile;
$jsonfile = $json->{file} || File::Spec->catfile($needledir, $json->{name} . '.json');
$jsonfile = $json->{file} || File::Spec->catfile($needles_dir, $json->{name} . '.json');
}

my $self = {};
Expand All @@ -61,16 +61,16 @@ sub new {
# - This code initializes $json->{file} so it contains the path within the needle directory.
# - $jsonfile is re-assigned to contain the absolute path the the JSON file.
# - The needle must be within the needle directory.
if (index($jsonfile, $needledir) == 0) {
$self->{file} = substr($jsonfile, length($needledir) + 1);
if (index($jsonfile, $needles_dir) == 0) {
$self->{file} = substr($jsonfile, length($needles_dir) + 1);
}
elsif (-f File::Spec->catfile($needledir, $jsonfile)) {
elsif (-f File::Spec->catfile($needles_dir, $jsonfile)) {
# json file path already relative
$self->{file} = $jsonfile;
$jsonfile = File::Spec->catfile($needledir, $jsonfile);
$jsonfile = File::Spec->catfile($needles_dir, $jsonfile);
}
else {
die "Needle $jsonfile is not under needle directory $needledir";
die "Needle $jsonfile is not under needle directory $needles_dir";
}

if (!$json) {
Expand Down Expand Up @@ -323,21 +323,21 @@ sub default_needles_dir {
}

sub init {
$needledir = ($bmwqemu::vars{NEEDLES_DIR} // default_needles_dir);
$needledir = File::Spec->catdir($bmwqemu::vars{CASEDIR}, $needledir) unless -d $needledir;
die "needledir not found: $needledir (check vars.json?)" unless -d $needledir;
$needles_dir = ($bmwqemu::vars{NEEDLES_DIR} // default_needles_dir);
$needles_dir = File::Spec->catdir($bmwqemu::vars{CASEDIR}, $needles_dir) unless -d $needles_dir;
die "needles_dir not found: $needles_dir (check vars.json?)" unless -d $needles_dir;

%needles = ();
%tags = ();
bmwqemu::diag("init needles from $needledir");
find({no_chdir => 1, wanted => \&wanted_, follow => 1}, $needledir);
bmwqemu::diag("init needles from $needles_dir");
find({no_chdir => 1, wanted => \&wanted_, follow => 1}, $needles_dir);
bmwqemu::diag(sprintf("loaded %d needles", scalar keys %needles));

if ($cleanuphandler) {
&$cleanuphandler();
}

return $needledir;
return $needles_dir;
}

sub tags {
Expand Down
10 changes: 5 additions & 5 deletions t/01-test_needle.t
Expand Up @@ -36,8 +36,8 @@ require tinycv;

my ($res, $needle, $img1, $cand);

my $data_dir = dirname(__FILE__) . '/data/';
my $misc_needle_dir = abs_path(dirname(__FILE__)) . '/misc_needles/';
my $data_dir = dirname(__FILE__) . '/data/';
my $misc_needles_dir = abs_path(dirname(__FILE__)) . '/misc_needles/';

$bmwqemu::vars{NEEDLES_DIR} = $data_dir;
needle::init;
Expand Down Expand Up @@ -381,7 +381,7 @@ subtest 'needle::init accepts custom NEEDLES_DIR within working directory and ot
my $needles_dir = $bmwqemu::vars{NEEDLES_DIR} = "$temp_working_dir/some-needle-repo";
make_path("$needles_dir/subdir");
for my $extension (qw(json png)) {
Mojo::File->new($misc_needle_dir, "click-point.$extension")->copy_to("$needles_dir/subdir/foo.$extension");
Mojo::File->new($misc_needles_dir, "click-point.$extension")->copy_to("$needles_dir/subdir/foo.$extension");
}

subtest 'custom NEEDLES_DIR used when within working directory' => sub {
Expand All @@ -397,7 +397,7 @@ subtest 'needle::init accepts custom NEEDLES_DIR within working directory and ot
};

subtest 'click point' => sub {
$needle::needledir = $misc_needle_dir;
$needle::needles_dir = $misc_needles_dir;

my $needle = needle->new('click-point.json');
is_deeply($needle->{area}->[0]->{click_point}, {xpos => 2, ypos => 4}, 'click point parsed');
Expand All @@ -412,7 +412,7 @@ subtest 'click point' => sub {
};

subtest 'workaround property' => sub {
$needle::needledir = $misc_needle_dir;
$needle::needles_dir = $misc_needles_dir;

my $workaround_string_needle = needle->new('check-workaround-bsc1234567-20190522.json');
my $workaround_hash_needle = needle->new('check-workaround-hash-20190522.json');
Expand Down
2 changes: 1 addition & 1 deletion t/02-test_ocr.t
Expand Up @@ -28,7 +28,7 @@ unless (which('tesseract')) {

needle::init;

my $img1 = tinycv::read("$needle::needledir/bootmenu.test.png");
my $img1 = tinycv::read("$needle::needles_dir/bootmenu.test.png");
my $needle = needle->new('bootmenu-ocr.ref.json');
my $res = $img1->search($needle);
ok(defined $res, 'ocr match 1');
Expand Down
4 changes: 2 additions & 2 deletions t/17-basetest.t
Expand Up @@ -220,8 +220,8 @@ subtest record_screenmatch => sub {

# check a needle has workaround property
my $basetest_for_workaround = basetest->new();
my $misc_needle_dir = dirname(__FILE__) . '/misc_needles/';
my $needle_file = $misc_needle_dir . "check-workaround-hash-20190522.json";
my $misc_needles_dir = dirname(__FILE__) . '/misc_needles/';
my $needle_file = $misc_needles_dir . "check-workaround-hash-20190522.json";
my %workaround_match = (
area => [
{x => 1, y => 2, w => 3, h => 4, result => 'ok'},
Expand Down
18 changes: 9 additions & 9 deletions t/21-needle-downloader.t
Expand Up @@ -23,8 +23,8 @@ $user_agent_mock->mock(get => sub {
});

# setup needle directory
my $needle_dir = $needle::needles_dir = path(tempdir, 'needle_dir');
ok(make_path($needle_dir), 'create test needle dir under ' . $needle_dir);
my $needles_dir = $needle::needles_dir = path(tempdir, 'needles_dir');
ok(make_path($needles_dir), 'create test needle dir under ' . $needles_dir);

subtest 'deduce URL for needle download from test variable OPENQA_URL' => sub {
$bmwqemu::vars{OPENQA_URL} = 'https://openqa1-opensuse';
Expand Down Expand Up @@ -70,22 +70,22 @@ subtest 'add relevant downloads' => sub {

# pretend that ...
# ... one file is already up to date (to the exact second)
File::Touch->new(mtime => 1514764800)->touch($needle_dir . '/foo.png');
File::Touch->new(mtime => 1514764800)->touch($needles_dir . '/foo.png');
# ... one file is present but outdated (by one second)
File::Touch->new(mtime => 1514764799)->touch($needle_dir . '/bar.json');
File::Touch->new(mtime => 1514764799)->touch($needles_dir . '/bar.json');

# define expected downloads: everything from @new_needles except foo.png
my @expected_downloads = (
{
target => $needle_dir . '/foo.json',
target => $needles_dir . '/foo.json',
url => 'http://openqa/needles/1/json',
},
{
target => $needle_dir . '/bar.json',
target => $needles_dir . '/bar.json',
url => 'http://openqa/needles/2/json',
},
{
target => $needle_dir . '/bar.png',
target => $needles_dir . '/bar.png',
url => 'http://openqa/needles/2/image',
}
);
Expand All @@ -95,7 +95,7 @@ subtest 'add relevant downloads' => sub {
sub {
$downloader->add_relevant_downloads(\@new_needles);
},
qr/.*skipping downloading new needle: $needle_dir\/foo\.png seems already up-to-date.*/,
qr/.*skipping downloading new needle: $needles_dir\/foo\.png seems already up-to-date.*/,
'skipped downloads logged'
);
is_deeply($downloader->files_to_download, \@expected_downloads, 'downloads added')
Expand Down Expand Up @@ -127,6 +127,6 @@ subtest 'download added URLs' => sub {
], 'right URLs queried');
};

remove_tree($needle_dir);
remove_tree($needles_dir);

done_testing;

0 comments on commit d867943

Please sign in to comment.