A recent TS config change elsewhere had caused these tests not to
compile with the error of:
```
error TS6307: File '/Users/jacktfranklin/src/devtools/devtools-frontend/scripts/component_bridges/value_for_type_node.ts' is not listed within the file list of project '/Users/jacktfranklin/src/devtools/devtools-frontend/test/unittests/scripts/component_bridges/tsconfig.json'.
```
The fix is to mark the dependency from the unit tests as a TS project
reference, and then generate output in the same place as the source, so
that import paths don't need to change.
This isn't ideal, and we would use ts_library if doing this now, but
this code pre-dates ts_library and also is only going to be around for
the length of the TypeScriptification work, so it doesn't feel worth the
effort to restructure it.
Change-Id: I9a924bf3e2ed945dd22ce233eda2c8c15f3193c6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2466188
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
This CL hooks up the collect-strings step with presubmit. Any UIStrings in the front_end javascript files will be gathered, and en-US.json and en-XL.json will be generated.
Note: This step has minimum impact on the presubmit performance (<1s)
DISABLE_THIRD_PARTY_CHECK = reason: removing extra loggings from the existing scripts.
Change-Id: I0065449766719a67d0ed604de3941bbc050b3780
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2409193
Reviewed-by: Vidal Diazleal <vidorteg@microsoft.com>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Christy Chen <chrche@microsoft.com>
If you have whitespace or a missing EOL, ESLint would automatically
fix these issues. However, the canned checks ran before ESLint. Thus,
the canned checks detected whitespace/EOL issues and would report
these problems. This would be confusing, but wouldn't be an issue in
isolation as you would have changed files that need to be checked in.
However, the presubmit then claims "Do you want to proceed regardless
of PRESUBMIT errors", as the canned checks can be suppressed by default.
This would then confuse engineers, as they are uploading, the presubmit
would report errors that had already been fixed and would (understandably)
skip the presubmit. They would then see CQ fail after a while, which
would be a while later.
R=jacktfranklin@chromium.org
Change-Id: I5bd55fbf441eed0fe1bcd37e87b03699c9774d40
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2409980
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This CL introduces a PRESUBMIT that will:
1. run `npm run regenerate-all-component-bridges`
2. Check that the git diff is clean.
If the git diff is not clean, it will fail. If the bridge has to be
manually edited (e.g. to work around a bug in the generator), it must
have a comment that matches the pattern `MANUALLY_EDITED_BRIDGE=...`.
The regeneration script will ignore bridges that contain these.
Change-Id: I372a3cd17a8e94bcdcea69ef64db189d2be6bde1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2364597
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The time of a regular presubmit time has increased. To ease debugging
now and in the future, add the durations of each individual step
to the logs.
Since _checkWithNodeScript and all non-Node steps rely on
_ExecuteSubProcess we only need to implement it there.
Also reorder the presubmit outputs such that we only print "Running
X check" when we actually run it.
R=petermarshall@chromium.org
Bug: 1116989
Change-Id: I1f30c3b5dadb961ebfbfef56d50bd9347e9069f2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2367941
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
On Windows, if your checkout is at:
C:\devtools\devtools-frontend
Then the stylelint CLI lives here:
C:\devtools\devtools-frontend\node_modules\stylelint\bin\stylelint.js
Prior to this patch, the lint CSS script was instead trying to use:
C:\node_modules\stylelint\bin\stylelint.js
That is, it went two levels too far up the directory tree. This only
happened on Windows.
This patch avoids the problem altogether by forcing the root directory
to be the current working directory and passing a root-relative glob
instead.
As a drive-by, this patch cleans up the `_getFilesToLint` helper in
`PRESUBMIT.py`.
Bug: chromium:1083142
Change-Id: Ida7d19e4c1cb8954346a3103ce95748f0bc38f00
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2252005
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
We ignore some files for ESLint, as they are either generated or
third_party. However, when someone tries to change these files,
ESLint would emit a warning stating that the file in question is
ignored.
This issue was reported to ESLint in https://github.com/eslint/eslint/issues/9977
The suggested workaround is to use the CLIEngine to filter out
the problematic paths. However, since our script was written in
Python, that API is not accessible to us.
Therefore rewrite the script to Node and filter out the problematic
files. The calls to CLIEngine were mostly taken from eslint/lib/cli.js,
which was the previous file used by `node_modules/bin/eslint`.
R=jacktfranklin@chromium.orgCC=sigurds@chromium.org
Change-Id: Iee600f0e0d99fcb6eeeb203a952a50fe35f9aaf3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2149316
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
For most of the PRESUBMIT invocations, the CL in question does not
change anything with regards to ESLint. Therefore, we can skip doing the
full check and only perform the check on relevant JavaScript and
TypeScript files.
However, if we do change the ESLint configuration (via one of the
involved build scripts/configuration files), we should run the full
check to make sure we are compliant.
`npm run check-lint` will still run the full lint check.
This saves about 15 seconds on a regular PRESUBMIT invocation, scaling
with the number of files changed.
R=jacktfranklin@chromium.org,aerotwist@chromium.org
Change-Id: I9c1bb055274bfa1d8cc47c9039a1196c92eba399
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2097993
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This improves the PRESUBMIT performance as we can rely on the AST
parsing of ESLint, meaning we don't have to parse AST twice.
It also allows us to use the nice `--fix` solution to insert the proper
license header in the files.
This shaves an expected 5 seconds of the presubmit time
(non-scientifically computed based on 2 uploads).
Change-Id: I5c53e9b232585f9ec46f86b36557c50bf80f7add
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2097992
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
The configuration was only including files that were in a commit diff.
This causes issues where we generate sources during the PRESUBMIT (which
are not in the git diff at that point in time) and don't format.
At the same time, we remove the complicated logic to reuse the
`.eslintignore` for clang-format purposes and use the correct
`.clang-format` "DisableFormat" options to disable formatting in various
directories.
Roll CodeMirror: ignore
Change-Id: I21b95a6e09c5d6bf036bf1aace103d6c26823ecc
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2091445
Reviewed-by: Changhao Han <changhaohan@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
This allows us to add an npm command that does the same thing. This is
necessary to support the workflow of making changes in the pdl during
development.
You can regenerate the relevant protocol files with
`npm run generate-protocol-resources`. Note that the files it generates
are not formatted, but on presubmit it will both format and lint the
files to the correct notation. However, that should not impact the
workflow of making changes to the protocol files and testing their
behavior.
R=sigurds@chromium.org
Change-Id: I87c06183c8842d8643c4647cb1519850d75bd46d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2088093
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
This checks in the InspectorBackendCommands in the source tree file
and ensures that it is kept in sync whenever the third_party
location has been updated.
In the process, I discovered multiple misconfigurations in the formatting
presubmit check. First of all, it was never running, because the
.eslintignore had an empty line. Second of all, it was running twice,
which is unnecessary since we now check for changed files at the end
of the presubmit. Lastly, it was only formatting JS files, while it
should check all files.
I have also updated the _CheckGeneratedFiles check to only run if it
is actually necessary. If there are no changes made to any affected of
the files, it will skip the step. This should thus reduce the presubmit
time and we will only pay the cost if we actually update any of the
files.
Lastly, it will now properly format and lint the generated files. This
makes reading the code a lot easier and makes it easier to digest the
diff when a protocol update goes through. I have verified that, in a
full build, the files are still minified. Thus, this has no impact on
the loading performance.
DISABLE_THIRD_PARTY_CHECK=Updating protocol generation
Fixed: 1056614
Change-Id: If49b0e749978ea1a7838992ec13507ee761ad76c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2087765
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
In presubmit.py, _CheckDevtoolsLocalization() gets all the paths of affected files and pass those in as arguments when running run_localization_check.py. When a lot of files need to be scanned, it casus the issue where character number exceed the limit of CreateProcess on Windows.
This PR changes the _CheckDevtoolsLocalization() to create a temp file that contains the affected file paths, and have the validation step read the affected file paths from the temp file.
Bug: 941561
Change-Id: I698e5064287eee8a8a3aca0682e3ab3521eb63d1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2032086
Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
Commit-Queue: Christy Chen <chrche@microsoft.com>
This reverts commit 402078e2f9.
Reason for revert: run_localization_check.py breaks the PRESUBMIT for any CLs which contain json files, i.e. module.json.
Original change's description:
> Adding run_localization_check.py
>
> It will run the two localization verifications:
> - It will tell if the resource files are formatted in the correct way
> - It ill tell if any localizable resource in the code is present with the
> appropiate attributes in the resource files.
>
> This two verifications already run as part of the presubmit verifications,
> I did some refactoring to allow them to be run from an external script.
>
> Change-Id: I340e5dcfdeef7331a757adf27cef94e22302cf10
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1894913
> Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
> Commit-Queue: Vidal Diazleal <vidorteg@microsoft.com>
TBR=lomitch@microsoft.com,vidorteg@microsoft.com
Change-Id: I0f2dfa1e071a0302414cf38ccbdf86ca772b3ec1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1926496
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Andres Olivares