I use the standalone devtools-frontend checkout process like most people
working on devtools, however I placed this repo inside chromium here:
D:\dev\chromium\devtools\devtools-frontend
This way I have both src and devtools-frontend side by side.
I just realized that this prevented me from running the css linter.
Indeed, run_lint_check_css.js tries to find the path to stylelint exe
file based on whether devtools is standalone or integrated in
chromium.src.
If it sees a directory named chromium inside the path, it assumes
devtools is integrated in chromium.src which, in my case, is wrong.
The change attempts to make this logic a little bit more safe by
checking a longer part of the path.
This resolves my issue.
Bug: 1198532
Change-Id: I872429da0751280bef5a4ec68602001421f55056
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2821855
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Brandon Walderman <brwalder@microsoft.com>
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
This CL fixes the fact that the stylelint rule wouldn't deal with:
```
border-bottom: var(--foo) solid var(--color-details-hairline)
```
It does this via a naive regex that splits the border value into its
three pieces, and then only lints the final declaration, which is the
color.
Bug: 1198504
Change-Id: I409adaf8b8777112ce4f3b24f0a6f63bd7435c25
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2823831
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
The component server needs to inject CSS and JS scripts into each
example. It was doing this by relying on a <style> and <script> tag as
the hook to inject more code. But if you have an example without a
<style> or <script> tag, it won't work. Instead we now inject based on
the </head> and </body> tags, which will always exist (or, if they
don't, we have bigger problems!)
Bug: None
Change-Id: Id2f586c917ee7ab0821d5ab8e50e5cd3d3647a92
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2815131
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The separate *_OWNERS files are related and can be grouped together
in a folder to make this distinction clear. This will allow us
to update the PRESUBMIT script to explicitly mark them as
exclusive change directory.
Note that the directory is named "owner", as we can't name it
"owners" as that is case-insensitive equivalent to "OWNERS",
which Windows can't handle in its presubmits.
DISABLE_THIRD_PARTY_CHECK=OWNERS update
R=yangguo@chromium.org
Bug: 1187573
Change-Id: Ib94a544c9dc01f788a8b75a5c8111ab52fbc27b8
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2814658
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
This CL adds a rule that ensures that any LitHtml.render call is
nested within a coordinator.write. It uses a fairly simple approach of
looking for a parent coordinator.write() call, but it should pick up most
cases.
Note this CL doesn't enable the rule; I'll do that in a follow-up. There
are 22 violations of the rule, so I will work on fixing those.
Bug: 1188116
Change-Id: Id73eaaa76db4685450086d2f77e7b2de322210d0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2799754
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Prior to this CL, if we gave the dark mode generation script a CSS
file that contained:
```
.foo {
color: var(--override-some-custom-color);
}
```
The script would generate the following CSS:
```
.-theme-with-dark-background .foo,
:host-context(.-theme-with-dark-background) .foo {
color: var(--override-some-custom-color);
}
```
But because the value of `color` is a variable, this CSS does nothing
that the original CSS won't do. Therefore the CL checks each rule for
any declarations that include `var(--` in them, and removes them from
the CSS that is sent to be dark patched.
Bug: chromium:1188524
Change-Id: I1257ac7c8b9f11426ce5c9b44bd89900072cf1b3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2797199
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
This CL fixes a bug where the test runner took the `--chrome-features`
flag but Conductor never read that value when booting up a browser
instance.
This CL updates it so it will read the value and pass it into the
relevant argument when executing `puppeteer.launch`.
I also changed the Python test suite runner to not prepend
`--enable-features`; that feels like something that Conductor should
do, not the test runner.
Bug: chromium:1186163
Change-Id: I05916d39d60bb98a606570cd82e688581dd78a40
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2790874
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This reverts commit 7742a201a1.
Reason for revert: We do have a use case for this flag that we missed, so we need it. Will follow-up with a CL to fix the fact that the test frontend infra doesn't use it currently.
Original change's description:
> [TestRunner] remove --chrome-features flag
>
> The scripts (both the old Python one and the new JS one) took
> `--chrome-features` as a flag, which was then set as
> `process.env.CHROME_FEATURES`, however searching the codebase for
> `CHROME_FEATURES` revealed that whilst we set this value, we never
> read it. Therefore this CL removes it entirely, my logic being that if
> someone needed this flag to work, we would have found this bug a long
> time ago.
>
> We can reintroduce should we find a usecase for it in the future.
>
> Bug: chromium:1186163
> Change-Id: I11d900df4aaad232fd64b9edca2aac12c94fc988
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782555
> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
> Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Bug: chromium:1186163
Change-Id: I336a4a14a3e720f3b0626d78148038ceab9a8774
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2786942
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
In release mode, the devtools_entrypoint prebundle step was missing
the deps. We ran into this issue when we were moving the emulation
panel into panels/emulation, which required some updates to the
toolbox.ts entrypoint (crrev.com/c/2782540).
The missing deps then required the rootdir logic to be changed,
such that deps are correctly resolved relative to the gen-directory,
not the source directory.
This CL might improve build performance in release builds, since
we are now properly using incremental references for our
entrypoints and saving a bit of compilation time.
DISABLE_THIRD_PARTY_CHECK=TypeScript fix
R=aerotwist@chromium.org
Bug: 1187573
Change-Id: I4662ac90ba50116cb668dfe3164fe725b74f9a37
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782554
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
The scripts (both the old Python one and the new JS one) took
`--chrome-features` as a flag, which was then set as
`process.env.CHROME_FEATURES`, however searching the codebase for
`CHROME_FEATURES` revealed that whilst we set this value, we never
read it. Therefore this CL removes it entirely, my logic being that if
someone needed this flag to work, we would have found this bug a long
time ago.
We can reintroduce should we find a usecase for it in the future.
Bug: chromium:1186163
Change-Id: I11d900df4aaad232fd64b9edca2aac12c94fc988
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782555
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
The dark mode generation was disabled due to a bug
(https://crbug.com/1185185) in the Sources Panel.
The source of that bug was that the dark mode generation takes all the
CSS in the source CSS file, applies the colour patching to it,
prepends each rule with `.-theme-with-dark-background` and then writes
the new file to disk.
The problem was that any CSS overrides defined in the source file
would not be applied, because the automatic CSS generated by the dark
mode script would be loaded after it and therefore take precedent. The
fix is to make the script smarter and recognise any overrides provided
in the source CSS file, and include them in the bottom of the dark
mode script, so that they are still applied and override the generated
CSS. This isn't ideal, as it means we load some blocks of CSS twice,
but it's better than having to manually update colours in some of our
CSS files that have 50+ declarations to update.
This CL doesn't re-enable the use of any of the dark mode scripts,
this will be done in a follow-up.
Bug: chromium:1188524
Change-Id: I03a1daa57d5535bb768696ca0d7bafc968618035
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2780804
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
When source code is in nested folders, the es_modules_import rule
would incorrectly compute the containing folders from `front_end`.
Thus, if you would have two sibling folders in `front_end/panels`,
the rule would incorrectly determine that they are in the same
folder, while technically they are in a different folder.
Therefore, the logic is rewritten to compute the common prefix
of the two paths. If either one path ends up resolving to just
a file, we assume that they were both part of the same folder.
In other words: iff after removing the common prefix you still
are importing both paths across a folder boundary, they are
considered a different folder.
R=jacktfranklin@chromium.org
Bug: 1187573
Change-Id: Ib34583e258e005a124735e17e6a92e829eb37ef3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2773054
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
The current tests try to figure out the root directory and then glob
for _test.ts files within there, but this breaks easily if the folder
structure isn't exactly as expected. Instead we can set this via a
flag in the test runner.
This CL only updates the interaction tests, which use the new runner.
e2e tests will be supported in time.
Bug: chromium:1186163
Change-Id: I198b67bf0081ce7334c40d7d661fb8642c2d56d0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2772045
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This CL adds a flag to the new test runner to provide the test-server-type.
We intelligently set it if the test suite ends with `e2e` or
`interactions`, but otherwise it's required. Unfortunately this can't
be checked as part of Yargs' API, so we check for it ourselves at run
time.
Bug: chromium:1186163
Change-Id: I6487f1a95c6c86759e1489309d4c18aec1292073
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2767266
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
That. Note that this currently isn't used or set because everyone +
all our scripts still run the Python version of the test runner
script, not the new script. But this needs to be in place before we
move people over to the new script.
We provide a fallback value such that this still works and is able to
run the tests despite the test runner not providing the configuration
via an environment variable.
Bug: chromium:1186163
Change-Id: I8255e5c7f9902be845e3c33add7b1703f6e4db9d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2766904
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
There are a handful of e2e tests which test regressions, and consequently
have a bug assiciation. This association was part of the test names in
the same way as we reference bugs for skipped tests. If such a test is
then skipped it needs to start with two crbug references. That's
confusing for the reader as well as a the skipped test lint check, for
which the regression bug references can lead to false negatives. This CL
introduces a new lint check that prevents bug references in non-skipped
tests with the same syntax as skipped tests.
Bug: none
Change-Id: Ibdf59091aba942f85424c1f12cc31160d9fe786a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2761357
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
This CL adds two flags to the new test runner for configuring the
components server and then updates the server to detect those values.
Note that whilst this CL adds yet-another-way to configure the
components server, this is only being done for backwards compat whilst
we roll out this change to all the various places. We will remove old
options towards the end of this work.
Bug: chromium:1186163
Change-Id: I4c9b85a5745b9fe36894e3c492952938a5b8f648
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2763875
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL lands the initial new run_test_suite.js that can take its
configuration from a config JSON file, or from flags.
This new script is only used to run `npm run auto-interactionstest`
and nothing more; I want to roll it out slowly and ensure that
everyone is aware of it before removing the old Python script. I will
create a doc with all the various steps, and required documentation,
as I've taken the chance to rename some options to make them clearer.
Bug: chromium:1186163
Change-Id: I5a199f82ac7ab0f323988acacaa7aae2d64e6349
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2763866
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This also updates the Runtime and build_release_applications to use
the correct format for the various files. Most notably, it should
use the basename when concatening file names, since we don't want
to include the `panels/` part in these file names. As such, the
filenames become `panels/accessibility/accessibility_module.js`.
R=aerotwist@chromium.org
Bug: 1187573
Change-Id: I2d3d453ac0e5ea0d4358228531bfe31bde6e7655
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2757147
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Tim van der Lippe