`waitForFunction` previously always waited 100 ms before checking if
the given function is truthy. We can improve the total e2e test suite
running time by 40-50 s by eliminating this timeout. This,
unfortunately, makes some of the tests flake since they relied on this
timeout. This CL attempts to eliminate the timeout and patch the tests
that relied on it. In the cases where a test could not be patched the
timeout is inserted in the test explicitly.
Local experiments showed that this CL reduces the total runtime from
on avg. ~309 seconds to an avg. of ~264 seconds.
This amounts to a ~45 second reduction or ~15%.
This is a reland of a26e3406c7
which was a reland of 3be2087f9d
Bug: 1112692
Change-Id: I3a5156d35553415bfbac95c0251b849ed77f4767
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2354096
Commit-Queue: Johan Bay <jobay@google.com>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
it.repeat() is helpful for fixing flaky tests.
Along with repeat, we also have the async tracing which is very helpful
for debugging tests that timeout due to hanging async operations. We
want to make all of these helpers available by default.
This CL introduces a new file mocha-extensions and adds both of these
customizations to the default mocha operations and then re-exports them.
In the future, all tests will import from mocha-extensions instead to
get these extensions by default. We will add lint rules to ensure that
callers don't import from mocha directly and miss out on these.
Bug: 1101782
Change-Id: I15dbb71493e60064288bfc050ac3f818a5c6821f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2339319
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
`waitForFunction` previously always waited 100 ms before checking if
the given function is truthy. We can improve the total e2e test suite
running time by 40-50 s by eliminating this timeout. This,
unfortunately, makes some of the tests flake since they relied on this
timeout. This CL attempts to eliminate the timeout and patch the tests
that relied on it. In the cases where a test could not be patched the
timeout is inserted in the test explicitly.
Local experiments showed that this CL reduces the total runtime from
on avg. 5:05 minutes to an avg. of 4:19 minutes.
This amounts to a ~46 s reduction or ~15%.
This is a reland of 3be2087f9d
Bug: 1112692
Change-Id: Id92542de85920f95bb922d1b62ddfb63b83a055a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2346366
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Changhao Han <changhaohan@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Commit-Queue: Johan Bay <jobay@google.com>
This CL also changes tests to favor `waitFor` over `$` per default:
In all tests where an element is expected `$` is changed to `waitFor`.
This should be uproblematic in all the cases where an element is
expected to exist immediately, but gives a bit of leeway in case the
element is slow to emerge. Typing wise, `waitFor` never returns null
since it throws if no element is found before it times out.
Change-Id: I00b722eff34df44402d9f3bf6786432c9faf1505
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2327710
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Changhao Han <changhaohan@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
AsyncScope objects allow tracking of the current stack trace during
chains of calls to test helpers. Recording stack traces allows us to
report pending helpers when a mocha timeout occurs.
The CL also introduces an AsyncTrace help that tests use to opt into
stack tracing. Wrapping the mocha it() call is necessary to hook into the
processing of the timeout. I export the AsyncTrace helper for clarity,
but we can also wrap mocha's it() directly into our own it() wrapper to
provide a drop-in replacement. AsyncTrace's it() allows configuring the
mocha timeout locally per test. We can chose to not do that and configure
it globally, or alternatively define standard timeouts for 'fast',
'regular', or 'slow' tests.
Change-Id: I9542cdfdd78a1868a9f0fa56b8d112bd549e6f34
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2328845
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
If we use a local timeout rather than letting this timeout the whole
mocha process, we get a way better error message which includes
info about the selector/function/whatever we care about, and the
step name. Without this we just get the test file name and nothing
else.
Change-Id: Ia2882fb123a6f2b582c0dd19f3f01135b496161a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2292276
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
(...in more than one place).
Lots of tests relied on the hosted mode server using port 8090 which
is not going to be the case once we have the parallel mocha runner
working.
Add the port number to puppeteer-state and have tests use it by
going through test/shared/helper.ts.
The port number is set at runtime rather than being a module-time
constant, so some users of the port needed to be changed to get it
after the root hooks ran instead. Future users should follow this
pattern and get the port in a before() block or during the test
itself. Or just use helpers that treat the port properly already
e.g. goToResource.
We don't send this port number to the hosted mode server yet,
so we still have this hardcoded in two places. A fix is coming in
a future CL.
Bug: 1101784
Change-Id: Ib0a7aa7a25db58d06624a1cdee6fa630cdfbe362
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2284828
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Changes include:
- Test server handles URLs with escaping better.
- Logging from DevTools frontend now logged by the test.
- Helper to turn on CDP logging - for debugging only.
- click helper improved to allow specifying maximum distance from
left, for when an element extends far beyond its containing
element.
- New pressKey helper that makes keyboard shortcuts easy.
- Helpers to modify and save a source pane and to get the text of
the line of code where we are stopped.
Bug: 1094436
Change-Id: I389eaa680bb0771a45104f470647f67b4fa5d1d9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2282811
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Eric Leese <leese@chromium.org>
This introduces a new goToResource shared helper for all e2e puppeteer
tests to use.
It helps simplify a bunch of tests and other helpers in that they don't
need to replicate the `${resourcesPath}/some/file.html` part.
Because this helper also knows how to get hold of the target on its own
it also further simplifies other tests that don't need to import it
anymore.
The next step would be to create a shared helper that knows how to
navigate to panels inside the DevTools front-end as this is also
duplicated across tests.
Bug: 1091226
Change-Id: I717455a51dec30e153d796ef101eab3d42bc2da1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2230320
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
Reviewed-by: Jan Scheffler <janscheffler@chromium.org>
Reviewed-by: Jose Leal <joselea@microsoft.com>
The shared test runner logic was reimplementing parts of Mocha,
in particular the test logging and filtering. Moreover, it was
booting the hosted mode server and puppeteer outside of Mocha.
Mocha supports root level hooks [1]. These hooks allow us to
perform work before a test starts. Moreover, by using the
before and after hook, we can run logic before and after
all tests.
By using these hooks, we can extract the "boot the hosted mode
server and hookup puppeteer" part to these root hooks.
Additionally, we can reset the pages in the `beforeEach`, which
means that tests themselves don't have to reset the pages.
We also put the implementation code into third_party/conductor,
as we would like to reuse this logic for the Puppeteer tests.
The Puppeteer test suite now also uses Mocha and has very
similar requirements as to our DevTools tests. By extracting
from DevTools, we can look into expanding the test runner to
other usecases, but that is out of scope for now.
[1]: https://mochajs.org/#root-level-hooks
Bug: 1071369
Change-Id: Ie9f954359d9de84da564b74b6f5517dd535db008
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2150458
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
We can use the native `fs.mkdirSync` instead as long as we set the
`recursive: true` option. This way, we don’t have to awkwardly pass
each path component as a separate array element, and we can now more
clearly represent paths as path strings throughout the code.
Change-Id: I83515d09a13844dccc41dd72943c59295aebdcb1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2108157
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
VS Code requires all files including .d.ts files to be included in order
to correctly resolve their definitions. However, we can't add the
include and exclude keywords to the tsconfig.json, as that would break
inheriting tsconfig.json configurations.
Therefore, extract a tsconfig.base.json with the original configuration
and let tsconfig.json extend it. This should instruct VS Code to use the
tsconfig, which includes all files, while all other infrastructure can
remain working as-is.
R=jacktfranklin@chromium.org
DISABLE_THIRD_PARTY_CHECK=TypeScript update
Fixed: 1056211
Change-Id: Ib4d127c745a3e69fd8c8c77eb0fb20c3a68689b1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2095299
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
We prefer single quotes by default, but still want to allow double
quotes in cases where that helps avoid quote-escaping, and also want
to allow template literals in cases where string interpolation is
used. For example:
const a = 'xxx'; // ok
const b = "xxx"; // not ok, should use single quotes
const c = "xxx'xxx"; // ok, double quotes avoid an escape sequence
const d = `xxx`; // not ok, frivolous use of template literal
const e = `xxx ${42}`; // ok
Per https://github.com/eslint/eslint/issues/12976, setting
the `allowTemplateLiterals` option for the `quotes` lint rule to
`false` gives us the desired behavior.
Note that the above lint rules are auto-fixed when running the linter;
there should be no need to manually make any changes to appease the
linter.
Per review feedback, this patch also removes the following escape
sequences for printable non-ASCII symbols:
- U+00D7 → ×
- U+2026 → …
- U+2019 → ’
Chromium CL temporarily updating test expectations:
https://chromium-review.googlesource.com/c/chromium/src/+/2083147
Cq-Depend: chromium:2083147
Bug: chromium:1057042
Change-Id: Id6bec3f96ca694d2fbc07dd8629fce305a58df8a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2082372
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Mathias Bynens <mathias@chromium.org>
Johan Bay