[cleanup] Move rebuild for test script parts [devtools/devtools-frontend : main]
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
desc: 'Deprecated. Do not use see warning bellow. Moved to `scripts/run_on_target.mjs`',Temporary allowed so the builders don't fail. Will remove once we update the builder scripts.
Related details
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 1 comment![]( "Open in Gerrit")
PTAL - this reuses the logic for rebuilding from `npm run build` and removes it from the `npm run test` command.
Related details
- Eric Leese
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 5 comments![]( "Open in Gerrit")
let sourceRoot = path.dirname(path.dirname(path.resolve(argv['$0'])));IIRC this was on purpose. `import.meta.dirname` is the physical directory of the file, which may not be the one we want in certain symlinked setups. `$0` holds where ths script was actually launched from.
const spinner = ora('Rebuilding...').start();How does logging work with this? Can we see the build output with this?
await build(target);Does this work in a chromium checkout?
[scriptPath, ...unparse(argv)],I don't think that does the right thing?
Related details
- Eric Leese
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nikolay Vitkov (Gerrit)
Nikolay Vitkov added 4 comments![]( "Open in Gerrit")
const spinner = ora('Rebuilding...').start();How does logging work with this? Can we see the build output with this?
I think we only see if anything fails else noting is piped to STDOUT.
Give that this is the current behavior of `npm start` `npm run build`, I think we should not block on this.
If we want that we should expose it on the `devtools_build` via a flag or ENV, but in a separate discussion.
await build(target);Does this work in a chromium checkout?
As stated this is the same as running `npm run build`, which Eric says works with a Chromium checkout.
[scriptPath, ...unparse(argv)],I don't think that does the right thing?
Refer to the `.parserConfiguration` - https://github.com/yargs/yargs-parser?tab=readme-ov-file#unknown-options-as-args
TLDR we consume what we expect and pass everything down to the next script.
'chrome',We lost these, haven't we?
Maybe. Eric does `npm run build` build this targets correctly, or do we need to fix that part first?
Related details
- Eric Leese
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 2 comments![]( "Open in Gerrit")
const spinner = ora('Rebuilding...').start();Nikolay VitkovHow does logging work with this? Can we see the build output with this?
I think we only see if anything fails else noting is piped to STDOUT.
Give that this is the current behavior of `npm start` `npm run build`, I think we should not block on this.
If we want that we should expose it on the `devtools_build` via a flag or ENV, but in a separate discussion.
Can't say I love this tbh, but fair enough.
[scriptPath, ...unparse(argv)],Nikolay VitkovI don't think that does the right thing?
Refer to the `.parserConfiguration` - https://github.com/yargs/yargs-parser?tab=readme-ov-file#unknown-options-as-args
TLDR we consume what we expect and pass everything down to the next script.
Yes, nonetheless I believe I did this intentionally, but I can't remember why. Did some quick testing and it appears to work in the relvant cases, so this LGTM.
Related details
- Eric Leese
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eric Leese (Gerrit)
Eric Leese added 1 comment![]( "Open in Gerrit")
'chrome',Nikolay VitkovWe lost these, haven't we?
Maybe. Eric does `npm run build` build this targets correctly, or do we need to fix that part first?
It doesn't build chrome, and I'm not sure we'd want it to. Not sure about the other targets; it looks like it just build devtools_all_files, which I recently discovered does not include the server and so isn't sufficient to run e2e tests. This isn't necessary in the npm start case, but running tests in a full checkout depends on building chrome.
Related details
- Nikolay Vitkov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |