Filing bug to summarize current proposal from email discussion.
Proposal is to en masse replace window.testRunner and window.internals with just testRunner and internals in all layout tests, and update docs noting best practices at:
https://cs.chromium.org/chromium/src/docs/testing/writing_layout_tests.md?q=writing_layout_tests.md&sq=package:chromium&dr&l=237
The only reason raised so far for the "window." prefix is to make it clearer to a new eng reading that it's a value injected by the test shell rather than something defined in local var or a library. Given we have plentiful test examples and documentation this value seems limited.
Today many tests look like:
if (window.testRunner)
window.testRunner.waitUntilDone();
Or:
if (window.testRunner)
testRunner.waitUntilDone();
After, tests would look like:
if (testRunner)
testRunner.waitUntilDone();
And similar for refs to internals.
Hack rough stats for consideration (note only counting html files, but there are some in js helpers as well):
Fully qualified testRunner:
% find . -name "*.html" | xargs grep -i "window.testrunner\." | wc -l
855
Shortened testRunner:
% find . -name "*.html" | xargs grep -i "testrunner\." | grep -v -i "window\.testrunner\." | wc -l
14904
Fully qualified internals:
% find . -name "*.html" | xargs grep -i "window.internals\." | wc -l
1285
Shortened internals:
find . -name "*.html" | xargs grep -i "internals\." | grep -v -i "window\.internals\." | wc -l
2401
Comment 1 by wkorman@chromium.org
, Mar 22 2017Two notes: 1. eventSender is another injected value to incorporate into this work. 2. window.XXX is preferred for if-conditions because it will not result in a JS RefError in console/logging when the value is undefined. Sample from JS console just now: > if (window.foo) { } undefined > if (foo) {} VM190:1 Uncaught ReferenceError: foo is not defined at <anonymous>:1:1