The helpers allow the implementation of screenshot tests in DevTools.
A screenshot test that makes use of the helpers is added for the
performance panel. The test is disabled because no screenshot test
infrastructure is available in our bots. Once this isn't the case the
test can be enabled and other screenshot tests added.
Screenshot tests are not added as a dedicated new suite but as
complementary to the interaction tests already in place. For this
purpose, the existing artifacts under test/screenshots/ are removed or
moved to be under test/interactions. The image_diff executables are
moved under third_party
Follow-up: Include dedicated lint rules files.
Bug: 1407638
Change-Id: I7106769b085b6aaa32ea4a089a840418c83536bb
DISABLE_THIRD_PARTY_CHECK=Including new library to third_party/
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4190012
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Andres Olivares <andoli@chromium.org>
The click helper now only accepts selectors so that it is
able to recover from unstable DOM situations. A new deprecated
helper clickElement is introduced for tests that cannot use a
selector currently. This should serve as a red flag that the
test might suffer from flakiness.
Bounding box and offset computation is removed for now. New
Puppeteer version allows providing relative click coordinates
so we need to use that.
Bug: 1411188, 1409768, 1411196, 1410437
Change-Id: I94a272184530842476bb2a879d862c4bd0930dba
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4194329
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
The attached bug highlights a case where navigating through a list of
computed styles is incredibly slow - on my machine, a single ArrowDown
keypress took >500ms.
We tracked the cause down to the `createPropertyElement` call, which is
called once per CSS property on each render. When a user clicks the
"Show all" button, this causes over 3,000 nodes to be rendered (one per
each style rule that could ever exist on a given element).
This means that this function needs to be performant and avoid doing any
expensive work. Inspecting the function revealed three things:
1. It was creating `ComputedStyleProperty` component instances via `new
ComputedStyleProperty()` calls. This means that a new instance will
be created on each render, even if all the data is identical.
Swapping to use lit-html template syntax (`<foo-bar ...>`) means that
Lit can manage the instance and not re-create it on each render call.
2. We were passing functions in as properties to the Lit component.
Because this function is created on each render, it ensures that Lit
will re-render every time as it considers the function to be
different.
3. Finally, the call to `renderValue()` in
`StylesSidebarPropertyRenderer` is expensive. Depending on the CSS
rule in question, it will create various DOM nodes and widgets (for
example, to render a colour picker). We were calling this on every
render, so all this HTML was being entirely recreated. This caused
heavy DOM thrashing.
To fix the above issues this CL makes a few changes:
1. We render `ComputedStyleProperty` via lit-html syntax. Additionally,
each property is passed in as its own setter, rather than a big `set
data`. Finally, rather than pass a function into the component, the
component dispatches an event which we can bind a listener too. This
is much more efficient as we don't have to re-bind the listener on
each render and do not have to re-render child elements because there
is now no function parameter that changes on each render.
2. We cache `renderValue()` and `renderName()` results based on the
property name and value. This ensures that A) we do the expensive
HTML generation only once, and B) that we don't change the HTML once
we've built it (unless the data changes, in which case we have no
choice).
With these changes the lag on navigation with the arrow keys is
completely gone, and a performance trace confirms we've drastically
reduced the amount of work being done and how long it takes.
There is still more that we could do here:
1. The `createTraceElement` function call has the same issues as
`createPropertyElement`. I've left this alone to keep the size of CL
down, and because I wanted to keep the scope small and focused on the
bug in question.
2. The `StylesSidebarPropertyRenderer#renderValue()` code could be
migrated to be a component, to avoid the issue of re-creating HTML.
Or, if that change is too large, we could introduce caching to it. As
far as I can tell the `propertyName` and `propertyValue` properties
of the class are never mutated, so once we create an instance of this
renderer and call `renderValue()`, the resulting output will never
change. As I'm not familiar with the code, I'd like to chat to people
who are (changhaohan@) after the New Year :)
3. The render-node callback that we invoke `createPropertyElement` from
seems to be doing a lot of work still. Ideally we would do most of
this work upfront and not on every render, but if we have to do the
work per-render we should explore what we can cache. Given the
slowness is when we render a list of 3000+ items, most of which do
not change between renders, I think there is likely wins to be had
with some clever caching or moving of code out of the re-render path.
Bug: 1403165
Change-Id: Ie6fb47b9f2e81e8f2a87c940370019f545fa50c8
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4110811
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Changhao Han <changhaohan@chromium.org>
In the color picker if there are sufficiently many colors in the
palettes, opening the palette switcher causes the switcher panel to be
rendered off screen because the color picker becomes scrollable. This
change scrolls the picker to the bottom to display the switcher panel
properly.
Fixed: 1365326
Change-Id: I4c61c92c5257284d5233b1fa2ef16ee090f2e255
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4020561
Reviewed-by: Ergün Erdoğmuş <ergunsh@chromium.org>
Auto-Submit: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
When tests time out we currently report the timeout error to resultdb/milo. This tells us nothing about where the test fails and authors/sheriffs need to scour the full text log for the error. Instead of the meaningless timeout error, this change reports the pending async stacks.
Bug: v8:13316
Change-Id: I0e1f1227aaaeb662b017a71ca082bf8311cab05d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3990724
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Change to the original CL:
* Updated `helper.ts` to use `pressKey` utility for summoning the search box. It wasn't been summoned before because `summon` function was using `Control` + `f` instead of `Meta` + `f` in MacOS.
This is a reland of commit b4643a4703
Original change's description:
> Add a global setting for search as you type
>
> Added a global setting to the settings screen and to the command menu
> for enabling search-as-you-type, or requiring Enter for searching,
> in search boxes that support it.
>
> - Added strings, a setting and commands for the new setting.
> - Bailed early on input when the setting is disabled.
> - Tests -
> - Added general helpers for getting text contents and summoning the
> search box.
> - Added elements-panel-specific helpers for the tests.
> - Made the setting toggle helper accept an optional expected state.
>
> Bug: 1344526
> Change-Id: I75612a54c10d5f40b208084fdbd0ffd7614f7f7c
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3859617
> Auto-Submit: PhistucK Productions <phistuck@gmail.com>
> Commit-Queue: PhistucK Productions <phistuck@gmail.com>
> Reviewed-by: Victor Porof <victorporof@chromium.org>
> Reviewed-by: Ergün Erdoğmuş <ergunsh@chromium.org>
Bug: 1344526
Change-Id: If92d692307a0ac298657ff9ace7df0c887fde1cf
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3865397
Auto-Submit: Ergün Erdoğmuş <ergunsh@chromium.org>
Reviewed-by: Victor Porof <victorporof@chromium.org>
Commit-Queue: Victor Porof <victorporof@chromium.org>
Added a global setting to the settings screen and to the command menu
for enabling search-as-you-type, or requiring Enter for searching,
in search boxes that support it.
- Added strings, a setting and commands for the new setting.
- Bailed early on input when the setting is disabled.
- Tests -
- Added general helpers for getting text contents and summoning the
search box.
- Added elements-panel-specific helpers for the tests.
- Made the setting toggle helper accept an optional expected state.
Bug: 1344526
Change-Id: I75612a54c10d5f40b208084fdbd0ffd7614f7f7c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3859617
Auto-Submit: PhistucK Productions <phistuck@gmail.com>
Commit-Queue: PhistucK Productions <phistuck@gmail.com>
Reviewed-by: Victor Porof <victorporof@chromium.org>
Reviewed-by: Ergün Erdoğmuş <ergunsh@chromium.org>
When clicking on elements, e2e tests compute the bounding box of
elements to click into. Some UI components update its contents by
removing and reinserting elements. The CL prevents the race that occurs
between elements being removed, and the e2e helper selecting an element
and running evaluate on it by looping until the selected element is
connected.
Fixed: 1349290
Change-Id: Ic9a0cc59a3c74ddbb2262ee20ba0360a3a2f643e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3834665
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
This CL is a preparatory refactoring. In the future, we want to load
DevTools from a different origin than `localhost` to increase
isolation between test runs (e.g. no shared window.localStorage).
To get there, we have to change how the extension tests load the
actual extension code. The extension must be served from the same
origin as the DevTools frontend. Currently this CL is a no-op, as
both resources and the DevTools frontend are served from 'localhost'.
R=pfaffe@chromium.org
Bug: 1297458
Change-Id: Iedee09d8cdd992f64132add78537428e9e4ab7ae
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3466036
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
This CL fixes a long-standing disabled l10n test. Initially we didn't
have a 'language' setting so we had to use the emulation feature
to simulate setting `navigator.language`. Finally, this is not how
we solved l10n for DevTools.
This CL adds a new helper that allows setting settings via the
Host API and uses it to make sure that the pseudo local is loaded
and applied correctly.
R=alexrudenko@chromium.org
Bug: 1163928
Change-Id: I1873ed38c9c6ef95ae87b281292e210363764211
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3466035
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
This is extracted from https://crrev.com/c/3330076/36
This is to fix interactions test failure like
```
1) shows cookies even when navigating to an unreachable page (crbug.com/1047348)
(node:3756728) Warning: Accessing non-existent property 'AsyncScope' of module exports inside circular dependency
at emitCircularRequireWarning (node:internal/modules/cjs/loader:707:11)
at Object.get (node:internal/modules/cjs/loader:721:5)
at waitFor (/usr/local/google/home/tikuta/devtools/devtools-frontend/test/shared/helper.ts:228:67)
at getElementPosition (/usr/local/google/home/tikuta/devtools/devtools-frontend/test/shared/helper.ts:57:21)
at click (/usr/local/google/home/tikuta/devtools/devtools-frontend/test/shared/helper.ts:104:13)
at navigateToApplicationTab (/usr/local/google/home/tikuta/devtools/devtools-frontend/test/e2e/helpers/application-helpers.ts:11:9)
at runMicrotasks (<anonymous>)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at Context.<anonymous> (/usr/local/google/home/tikuta/devtools/devtools-frontend/test/e2e/application/cookies_test.ts:29:5)
```
when we use esbuild.
Bug: 1278663
Change-Id: I9e3ab2d19c4daa56c87e3f77ac917a13d32a7c24
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3394445
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Tim Van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
The click event only fires for elements that receive both a mousedown
and a mouseup event. Since DataGrid elements in the Network panel get
replaced every time a message is received on WebSockets, the click
event doesn't get fired if you click before a message and release after
the message. To fix this we'll listen for the mousedown event instead.
This has the added benefit that the request panel becomes visible
sooner, making it feel more responsive.
Fixed: 1222382
Change-Id: Ia44c47a90b4822530081caf63aec682e9f32186c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3293092
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
This change is required to prevent a test timing out when we await the
coordinator being done when actually that work is already done.
Previously the helper would add a listener for the `rendererqueueempty`
event but if the work had been completed by then, the event would never
fire and the test would time out.
Now we expose globally a function to return the count of pending events
and the interaction/e2e test helper will check if that returns `0`, and
if so, resolve, before then adding the event listener.
This means a test that waits for work where that work has actually
already happened will now correctly resolve and continue, whereas before
it would time out indefinitely.
Bug: none
Change-Id: I43243bcc9002c9e97f476a66ed3f42c7313f7fcd
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3198872
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
`assertNotNull()` of `undefined` does not cause the test to fail,
whereas `assertNotNullOrUndefined()` does.
Because of import restrictions, platform.js's
`assertNotNullOrUndefined()` is copied to helper.ts.
Bug: 1072335
Change-Id: Iaa56492c2f58f735cb93b434f6a73c65cdb740c5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3057046
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Wolfgang Beyer <wolfi@chromium.org>
When an e2e test times out, running promises aren't aborted. Instead,
any errors that are thrown are silently swallowed. When not being
careful, in-flight promises can still mess up state, or even never
detect timeouts. waitForFunction is especially vulnerable to this,
because it's running an infinite loop inside the test process. This CL
extends AsyncScopes to carry a flag when a timeout was triggered, which
allows waitForFunction to bail.
Bug: none
Change-Id: I30ebcc45419c4000a1b2a25096ca1211bf2400a0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2978374
Auto-Submit: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Alex Rudenko