Summary: | JavaScriptCore tests should have a mode to use a shared jsc process | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Christopher Reid <chris.reid> | ||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | beidson, commit-queue, don.olmstead, fpizlo, ggaren, Hironori.Fujii, keith_miller, lforschler, mark.lam, msaboff, ossy, saam | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Other | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Christopher Reid
2016-10-21 11:48:27 PDT
An alternative solution would be to write a client and server wrapping jsc. This would avoid having to re-write the test scripts with ruby but it's adding a potentially unwanted layer of complexity. Are there any preferred approaches to this? DumpRenderTree accepts a series of file names through stdio and runs them sequentially. I think it would be fine to add a command-line option for that to jsc.cpp. I'm not sure how to solve your broader problem of not being able to launch processes for the scripted testing harness. Our test harness is pretty strongly designed around being able to launch processes. Created attachment 295764 [details]
Patch
The batch mode param is right now a lot more coupled in to the jsc-tests than I would like. I was mainly wanting some initial feedback on what would like to be seen.
Attachment 295764 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/jsc.cpp:2856: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2867: Missing spaces around = [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2867: Missing spaces around < [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/jsc.cpp:2891: Missing spaces around = [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/jsc.cpp:2891: Missing spaces around < [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/jsc.cpp:3180: Extra space before ) [whitespace/parens] [2]
Total errors found: 6 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 295764 [details]
Patch
This is a very invasive change to the test harness, and most JSC developers have platforms that have fast process start. We rely on the fact that tests are isolated in a lot of subtle ways, including relying on shell scripts that wrap each test. I don't want to lose this invariant in the test harness, because it will make all of our other test hacking harder.
Comment on attachment 295764 [details]
Patch
Patches that have been up for review since 2016 are almost certainly too stale to be relevant to trunk in their current form.
If this patch is still important please rebase it and post it for review again.
Closing this as it is no longer relevant. |