Fix skipping Selenium tests

[AdamWill]

Not sure what changed, but on current Fedora Rawhide, just 'exit'ing like this now results in a 255 exit code and a failed test. Before:

Install Selenium::Remote::Driver and Selenium::Chrome to run these tests
./t/ui/27-plugin_obs_rsync_status_details.t ................. ok
Install Selenium::Remote::Driver and Selenium::Chrome to run these tests
./t/ui/28-keys_to_render_as_links.t ......................... ok All tests successful.

after:

Install Selenium::Remote::Driver and Selenium::Chrome to run these tests
./t/ui/27-plugin_obs_rsync_status_details.t ................. ok
Install Selenium::Remote::Driver and Selenium::Chrome to run these tests
./t/ui/28-keys_to_render_as_links.t ......................... skipped: (no reason given) ...
./t/ui/28-keys_to_render_as_links.t (Wstat: 65280 (exited 255) Tests: 0 Failed: 0)
Non-zero exit status: 255

This uses plan skip_all to skip these tests correctly. We also have to reorder one test slightly so we skip before running any tests.

[AdamWill]

Huh. Well, the failure is in a file touched in the PR, but it doesn't immediately make any sense to me...

[23:19:23] t/ui/27-plugin_obs_rsync_status_details.t .. 1/? 
#   Failed test 'sub process openqa-webapi terminated with exit code 10752'
#   at /home/squamata/project/t/ui/../lib/OpenQA/Test/Utils.pm line 234.
# Looks like your test exited with 10752 just after 2.

[Martchus]

The more detailed logs (visible via the artifacts tab in CircleCI) are only slightly more helpful:

# Starting WebUI service. Port: 38945
# PID of openqa-webapi: 2843
[error] [pid:2843] Stopping openqa-webapi process because a Perl warning occurred: Failed to open /home/squamata/project/t/ui/../etc/openqa/database.ini: No such file or directory at /home/squamata/project/lib/OpenQA/Schema.pm line 40.
not ok 1 - sub process openqa-webapi terminated with exit code 10752
ok 2 - no (unexpected) warnings (via done_testing)
1..2

So with your change it would still attempt to start the web UI process. Maybe a combination of plan skip_all … and exit works?

But it is very strange because this test doesn't even seem to use Selenium so it shouldn't be affected by your changes at all. So I retriggered the test. Maybe the failure is a sporatic one and not related to your changes. I confused the test with t/ui/27-plugin_obs_rsync_obs_status.t. The problematic test uses Selenium.

---

EDIT: It failed very quickly with

  Error pulling image registry.opensuse.org/devel/openqa/ci/containers/base:latest: Error response from daemon: received unexpected HTTP status: 503 Service Unavailable... retrying
  image is cached as registry.opensuse.org/devel/openqa/ci/containers/base:latest, but refreshing...

which is definitely unrelated. I'll retrigger it later again.

---

EDIT: After retrying again the failure is reproducible.

[AdamWill]

That test does use selenium, the next line right after the driver_missing call is $driver->find_element_by_class('navbar-brand')->click;, for instance.

It's the test I had to reorder slightly in the PR - I had to make the setup_obs_rsync_test call come after the driver_missing call, because setup_obs_rsync_test uses a couple of test functions and so it's considered to have "run two tests". You can't use skip_all after running any tests.

Possibly the problem is with doing setup_obs_rsync_test after call_driver, though I don't know how. I guess I'll try rejigging it - I'll put the order back as it was, but have just this test not use driver_missing but instead do its own custom skipping (along the lines of how the old driver_missing worked) to account for the fact that two 'tests' have run already.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c944acc) 98.32% compared to head (028fa21) 98.32%.
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5349   +/-   ##
=======================================
  Coverage   98.32%   98.32%           
=======================================
  Files         389      389           
  Lines       37319    37320    +1     
=======================================
+ Hits        36693    36694    +1     
  Misses        626      626           

Files	Coverage Δ
t/lib/OpenQA/SeleniumTest.pm	99.35% <ø> (ø)
t/ui/27-plugin_obs_rsync_status_details.t	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kalikiana]

Please keep this within driver_missing and make it configurable

[AdamWill]

OK, so when I came back to this after the discussion at Test-More/test-more#919 , I came up with this idea: bail early in the troublesome test after all, but do it by using check_driver_modules instead of call_driver. I'm hoping this will be a nice still-simple-ish way to deal with the issue.

haarg's suggestion to use Test::Needs is an interesting one - we could look at dropping driver_missing and check_driver_modules and just using Test::Needs in all the relevant tests instead. but that seems like a bigger change that can be a separate PR.