-
Notifications
You must be signed in to change notification settings - Fork 180
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
QE: handle errors occurring during a page reload #8110
QE: handle errors occurring during a page reload #8110
Conversation
👋 Hello! Thanks for contributing to our project. If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code. Reference tests: KNOWN ISSUES Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience. For more tips on troubleshooting, see the troubleshooting guide. Happy hacking! |
631c446
to
7aed23f
Compare
Thanks for the effort Paolo. It will improve drastically our comfort of life to have this screenshot back ! |
I don't like the catching and re-throwing of errors but ATM it should to the work and avoid introducing further complexity in |
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.
just one noobish question
7aed23f
to
b0ca216
Compare
end | ||
rescue ScriptError, TimeoutError => e | ||
# an error may be raised while the page still (re)loads, making no screenshot available | ||
find('#page-body', wait: 3) |
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.
Maybe instead of waiting here, we should be waiting in the After method, just before we take the screenshot.
But in any case, we are already waiting some time before taking any screenshots, probably even too much...!
https://github.com/uyuni-project/uyuni/blob/master/testsuite/features/support/env.rb#L138
As we are waiting twice the default_max_wait_time.
If we look at has_content APIDoc:
https://www.rubydoc.info/github/jnicklas/capybara/master/Capybara%2FNode%2FMatchers:has_content
@nodeg FYI ^
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.
Right, it's much better indeed to have this kind of stuff in the After method.
Regarding the waits: when the failure happens in cases where we are not dealing with bootstrapping minions (as when checking the events history), this and condition will probably short circuit due to lazy evaluation.
click_button('Details') if has_content?('Bootstrap Minions') && has_content?('Details')
Maybe we can check for the exception which lead to a failure and use it to determine whether to force another wait, or not.
Something like:
find('#page-body', wait: 3) if scenario.exception.is_a?(TimeoutError)
Assuming the problem is actually not having enough time for the page to load.
Hi @NamelessOne91 , what is the status of this PR? |
Hi @maximenoel8 I'm not sure the issue is just not having enough time to reload the page before a screenshot is taken, see Oscar's comment. |
b0ca216
to
3d0e29a
Compare
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.
Once Rubocop issue is fixed, LGTM.
3d0e29a
to
66d01f3
Compare
What does this PR change?
Introduce a reusable function to reload pages and handle the case in which a TimeoutError is raised during a page reload, resulting in no screenshot being available for a failed scenario.
GUI diff
No difference.
Documentation
No documentation needed: only internal and user invisible changes
DONE
Test coverage
Cucumber tests were edited
DONE
Links
Issue(s): https://github.com/SUSE/spacewalk/issues/22208
Backport Manager 4.3: https://github.com/SUSE/spacewalk/pull/23602
Changelogs
Re-run a test
If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run: