Making test-functions strict required some test adjustments where we
incorrectly expected an number while we receive in fact a string. As
getAttribute() returns either a string or null.
This is supposed to behave similar to `cockpit.location.go()`. Our tests
always call this with either a hash or a path, and these are the only
two cases we support. Make that explicit with assertions, error
messages, and documentation.
Also clean up the test-functions.js backend: Replace `ph_go()` (which
covered both cases) with `ph_jump()` which only switches between pages
(like `cockpit.jump()` does, i.e. like clicking in the shell menu). Lift
the handling of hashes into `Browser.go()` directly; keep the current
implementation for now.
With a simple :contains() implementation cockpit-files and
subscription-manager can run their tests without requiring sizzle in
node_modules.
Note that this still allows passing elements after contains() such as
for example `:contains() input`, but it won't support them. This is
needed to support `b.click()` which appends `:not(:disabled)` CSS
selectors. A future addition would be supporting selectors after
`:contains()` so cockpit-machines can be supported.
This changes the behaviour from checking the visibility and display for
the element itself to checking if the element is inherently or directly
hidden by using `Window.getComputedStyle()`.
If a parent of the element we're checking has `visibility=hidden` our
previous check would have failed if not for our custom CSS that made
`hidden` elements get `display=none` applied. As we removed that in a
prior commit we now found the bug and need to correct it.
`Window.getComputedStyle()` will take all the values that the browser
itself would inherit. This coincidentally also means we are testing
closer to what users would experience in their own browser.
See-also: https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle
Related-to: e89892b11185529721b2c0ad7f5ee4a4b558a29a
Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
Huge team-effort to redesign Cockpit with Patternfly v6 in mind and to
reduce the amount of hacks and legacy code we have. This also paves the
way to further improve Patternfly itself by keeping up-to-date with
their latest releases.
Commit includes codemod changes, updated tests, improved layout for
desktop and mobile, as well as inline-comments here and there for what
we need to fix.
Co-authored-by: Garrett LeSage <garrett@redhat.com>
Co-authored-by: Tomas Matus <tomatus777@gmail.com>
Co-authored-by: Martin Pitt <mpitt@redhat.com>
Relates-to: #21651, #20973https://issues.redhat.com/browse/COCKPIT-1266
Add a `querySelectorAll()` variant which can transparently pierce and
traverse shadow DOMs without having to know their structure (i.e. doing
selectors piece-wise and iterating on their `.shadowRoot` attribute).
As this is more expensive than standard querySelector()`, only use it
if the page actually contains any shadow root.
This makes it a lot more convenient to write tests for pages which use
web components, like https://patternflyelements.org.
Our years-old `ph_mouse()` helper is cheating: It completely side-steps
the browser UI and directly synthesizes `MouseEvent`s in JavaScript.
This allowed funny things like clicking on a main page element while a
dialog is open (which you can't normally do as the dialog is modal),
clicking an element which is disabled in some non-standard way, or
clicking through a tooltip.
Make this more realistic by using the BiDi API for synthesizing mouse
events. This works fine with Firefox, but is unfortunately completely
broken in iframes with Chromium:
https://issues.chromium.org/issues/359616812
Specifying precise coordinates is not currently implemented.
`ph_mouse()` always clicks on the top left corner, while webdriver's
`pointerMove` puts (0,0) in the center of the element. This can be done
with a little extra calculation of the `getBoundingRect()`, but only
very few tests like `TestStorageLvm2.testMaxLayoutGrowth` rely on that.
For these tests, continue to use `ph_mouse()`.
https://issues.redhat.com/browse/COCKPIT-1158
webdriver's and BiDi's functions for preloading a script only supports
declaring and calling a function. This can't define globals.
So do what sizzle does and put the helper functions on the `window`
global object.
Drop the long-obsolete jQuery comment.
The tricky CSS tricks turned out to be a bit too tricky in the case
where the production CSS already uses ::after or ::before pseudo
elements. Our ListingTable component uses those alot for handling the
mobile layout, for example.
Let's use to most straightforward way: setting the text of an element
via injected JavaScript.
This might fail when React decided to render inbetween the call to
set_mock and capturing of the actual screenshot. But the tests keep
the pages quiet during pixel testing anyway, so hopefully this will
not happen in practice.
Another way this might fail is that React gets confused the next time
it renders after the call to set_mock because the DOM has changed too
much. This can hopefully be prevented by only replacing the text of
elements, and not their children.
This commit introduces a new pixel test scenario for RTL (Right to Left). This
scenario keeps using English for language but changes the direction of the language
to be RTL.
The reasoning behind this choice is that pixel tests take screenshots in the
middle of tests execution, by dynamically adopting the theme, browser
size etc for the different scenarios.
However in order to change the UI language one needs to reload the
page, which effectively means losing the application state. For this
reason, we would need to set the language at the very beginning of
each test if we wanted to properly test RTL with a language like Hebrew.
Since this means making a whole test run in another language, we decided
that we will for the time being test RTL with English.
This is obviously an unrealistic scenario but allows us to view spacing
issues at least.
Some code, like in user accounts dialogs, were using
`replace("\n", " ")`. This only replaces the first match.
Consistently use replaceAll() for all cases that replace static strings.
A lot of places were using `/haystack/g` which works correctly, but
replaceAll() with a static string is faster.
Spotted by CodeQL "Incomplete string escaping or encoding" alerts.
Disable `no-unused-vars` rule for this file; ESLint complains about all
of our API, like:
error 'ph_count_check' is defined but never used no-unused-vars
As this is not an ESM module, we can't use `export`.
Throw proper Error objects instead of literals:
error Expected an error object to be thrown no-throw-literal
ph_wait now allows all exceptions and keeps trying. If the last try
has an exception, it will be reported. This means that we don't lose
any exceptions that happen consistently, but we always have to wait
for the timeout.
Browser.wait_js_cond retries when the execution context is not found
or disappears in the middle. To support this, getFrameExecId never
raises an exception itself but returns -1 so that Runtime.evaluate
raises an error that causes Browser.wait_js_cond to retry.
Together these changes mean that calls to functions like
Browser.wait_in_text can now survive a page reload in their middle.
Consequently, all calls to expect_load and expect_load_frame have been
removed.
Specifically, this fixes a race in the TestKeys.testAuthorizedKeys
test case. That test case experiences a page reload that is triggered
asynchronously and can therefore not be reliably waited for with
expect_load_frame. (The call to enter_page was essentially a no-op.)
test/common/testlib.py has a set of layouts and test code can switch a
Browser into one via Browser.set_layout. Right now there is only
"desktop" and "mobile", but more can be easily added.
Browser.assert_pixels cycles through all the defined layouts and does
a pixel test for each.
TEST_MOBILE and cdp.mobile have been removed.
It's useful to wait for an attribute to exist. In that case,
`ph_attr_contains()` previously failed with
TypeError: Cannot read property 'indexOf' of null
So additionally wait for the attribute to exist in the first place.
`mouse` function had option `force` which made it possible to click on
invisible buttons. However this is a bad practice.
This was in the code mostly because in `check-active-pages` the dropdown
menu was needed to be opened while holding `alt` key down.
This commit teaches the mouse function to also simulate holding down
special keys.
In three other tests `force` was used pointlessly.
Closes#11812
We consider an element visible when at least one of width or height is
non-zero.
We frequently seem to get elements with zero height that are
nevertheless fully functional. For example, the "body" element itself
seems to be stuck at a zero height sometimes.
Install a Chrome Devtools Protocol handler for unhandled exceptions, and
fail the next CDP command with it. This got lost with the move from
PhantomJS to Chrome.
Some of our tests rely on failed ph_* function invocations, so we must
not log failures of ph_wait_cond(); these are also technically
unhandled exceptions. So turn this into a proper `PhWaitCondTimeout`
error class, so that it can be identified reliably.
Handle cases where undefined value is passed to fail/success in
cdp-driver.js.
Adjust the naughty override for the XTerm.js crash (#9641) as this now
shows the actual underlying crash instead of just the followup element
wait timeout.
Closes#9639
