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
Check unregistered needles when failing assert_screen #715
Conversation
needle.pm
Outdated
my $hash; | ||
for my $key (qw(tags properties area file png unregistered name)) { | ||
$hash->{$key} = $self->{$key}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kraih would probably recommend
my $hash = map { $_ => $self->{$_} } qw(tags properties area file png unregistered name));
or something along these lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i would. 😃 (my %hash = map {...} qw(...)
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually it burned in my fingers :)
return $goods || []; | ||
} | ||
my @results; | ||
|
||
# now check that it contains all the other tags too | ||
NEEDLE: for my $n (@$goods) { | ||
for my $t (@tags) { | ||
for my $t (@wanted) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for replacing the ambiguous @tags
within the method tags
. I got confused by it already :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, now there's only the tags()
method and the %tags
hash ;)
backend/baseclass.pm
Outdated
my ($foundneedle, $candidates) = $img->search(\@unregistered_needles, 0, 1, undef); | ||
# the best here is still a failure, as unregistered | ||
push(@$failed_candidates, $foundneedle) if $foundneedle; | ||
push(@$failed_candidates, @$candidates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, that's basically what I wanted to say today in the call. Just add some diag
and we already have something useful :-)
Changes Unknown when pulling a812a3e on coolo:more_candidates into ** on os-autoinst:master**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but i'd like to know the real poo# this PR addresses
It's poo#820 - there is just a s missing in the expanded url |
this seems to miss a commit or two. At least unregistered is unused |
but what is there is harmless so merging this |
https://progress.opensuse.org/issue/820