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

Simplify OpenQA::WebAPI::Controller::Test #4968

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

okurz
Copy link
Member

@okurz okurz commented Dec 22, 2022

No description provided.

@okurz okurz marked this pull request as draft December 22, 2022 13:00
@okurz
Copy link
Member Author

okurz commented Dec 22, 2022

tests fail with

[12:53:44] t/ui/02-list-group.t ....................... 1/? executeScript: unexpected alert open: {Alert text : DataTables warning: table id=running - Ajax error. For more information about this error, please see http://datatables.net/tn/7} at /home/squamata/project/t/ui/../lib/OpenQA/SeleniumTest.pm:81 at /home/squamata/project/t/ui/../lib/OpenQA/SeleniumTest.pm line 84.
	OpenQA::SeleniumTest::__ANON__(Test::Selenium::Chrome=HASH(0x55ef62a54968), "Error while executing command: executeScript: unexpected aler"..., HASH(0x55ef62a545a8), HASH(0x55ef62d616b8)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 356
	Selenium::Remote::Driver::catch {...} ("Error while executing command: executeScript: unexpected aler"...) called at /usr/lib/perl5/vendor_perl/5.26.1/Try/Tiny.pm line 123
	Try::Tiny::try(CODE(0x55ef62d63030), Try::Tiny::Catch=REF(0x55ef62d68558)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 361
	Selenium::Remote::Driver::__ANON__(CODE(0x55ef627c5d20), Test::Selenium::Chrome=HASH(0x55ef62a54968), HASH(0x55ef62a545a8), HASH(0x55ef62d616b8)) called at (eval 1712)[/usr/lib/perl5/vendor_perl/5.26.1/Class/Method/Modifiers.pm:89] line 1
	Selenium::Remote::Driver::__ANON__(Test::Selenium::Chrome=HASH(0x55ef62a54968), HASH(0x55ef62a545a8), HASH(0x55ef62d616b8)) called at (eval 1714)[/usr/lib/perl5/vendor_perl/5.26.1/Class/Method/Modifiers.pm:148] line 2
	Selenium::Remote::Driver::_execute_command(Test::Selenium::Chrome=HASH(0x55ef62a54968), HASH(0x55ef62a545a8), HASH(0x55ef62d616b8)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 1058
	Selenium::Remote::Driver::execute_script(Test::Selenium::Chrome=HASH(0x55ef62a54968), "return window.jQuery \x{26}\x{26} jQuery.active === 0") called at /home/squamata/project/t/ui/../lib/OpenQA/SeleniumTest.pm line 163
	OpenQA::SeleniumTest::wait_for_ajax("msg", "wait for test list without group") called at t/ui/02-list-group.t line 21
[12:53:44] t/ui/02-list-group.t ....................... 2/? # Tests were run but no plan was declared and done_testing() was not seen.
[12:53:44] t/ui/02-list-group.t .......................      Dubious, test returned 254 (wstat 65024, 0xfe00)
All 2 subtests passed 
[12:54:06] t/ui/03-source.t ........................... # Premature connection close
[12:54:06] t/ui/03-source.t ........................... 1/? 
#   Failed test 'GET /tests/99938/modules/installer_timezone/steps/1/src'
#   at t/ui/03-source.t line 20.

#   Failed test '200 OK'
#   at t/ui/03-source.t line 20.
#          got: undef
#     expected: '200'

#   Failed test 'installer_timezone test source found'
#   at t/ui/03-source.t line 20.
#                   ''
#     doesn't match '(?^i:installation/.*installer_timezone.pm)'

}
return $match;
sub get_match_param ($self) {
return unless my $match = $self->param('match');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return unless my $match = $self->param('match');
return undef unless my $match = $self->param('match');

We should always return something. If I'm reading it correctly undef is what we want here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and looking at the usage this is very important here because we use the method in list context

@@ -22,25 +22,16 @@ use constant DEPENDENCY_DEBUG_INFO => $ENV{OPENQA_DEPENDENCY_DEBUG_INFO};

sub referer_check ($self) {
return $self->reply->not_found if (!defined $self->param('testid'));
my $referer = $self->req->headers->header('Referer') // '';
return 1 unless $referer;
return undef unless my $referer = $self->req->headers->header('Referer') // '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it would always have returned 1. Maybe this is somehow causing the tests to fail?

}
return $match;
sub get_match_param ($self) {
return unless my $match = $self->param('match');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would now also return early if the parameter contains 0.

@mergify
Copy link
Contributor

mergify bot commented Mar 8, 2023

This pull request is now in conflicts. Could you fix it? 🙏

@stale
Copy link

stale bot commented Jun 10, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants