Bug 195265

<rdar://problem/47040673>

Created attachment 363484 [details] Patch

Comment on attachment 363484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363484&action=review > Source/WebInspectorUI/UserInterface/Models/AuditTestBase.js:106 > + async setup() I'm on the fence as to whether this should bump the `Audit.version`. On one hand, it's new functionality that should be able to be feature-checked. On the other hand, this is a purely additive change and won't affect any existing ability to run code. I'm leaning towards bumping the version, as otherwise an audit written with the expectation of being able to use `setup` would most likely fail with many errors (e.g. functionality defined in the `setup` would be missing).

Created attachment 363491 [details] Patch

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

Created attachment 363529 [details] Patch

Comment on attachment 363529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363529&action=review Awesome! r=me > LayoutTests/inspector/audit/manager-start-setup.html:86 > + description: "Check that Audit.run is able to respond with a RemoteObject.", Some of these descriptions are a little generic given this is testing nested behavior. > LayoutTests/inspector/audit/manager-start-setup.html:98 > + async test() { > + const setupA = (function() { > + WebInspectorAudit.__test = "A"; > + }).toString(); > + > + const setupB = (function() { > + WebInspectorAudit.__test = "B"; > + }).toString(); > + > + let audit = new WI.AuditTestGroup("AuditManager.prototype.start.OverriddenLevelSetup.Group", [ > + new WI.AuditTestCase("AuditManager.prototype.start.OverriddenLevelSetup.Test", auditTestString, {setup: setupA}), > + ], {setup: setupB}); Do I understand correctly that both of these setup's ran, and "A" ran first and was overridden by "B". So our chain is only the Top Level setup, and the per-test Setup, but no additional groups in between?

Comment on attachment 363529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363529&action=review >> LayoutTests/inspector/audit/manager-start-setup.html:86 >> + description: "Check that Audit.run is able to respond with a RemoteObject.", > > Some of these descriptions are a little generic given this is testing nested behavior. Copypasta :P Will fix. >> LayoutTests/inspector/audit/manager-start-setup.html:98 >> + ], {setup: setupB}); > > Do I understand correctly that both of these setup's ran, and "A" ran first and was overridden by "B". > > So our chain is only the Top Level setup, and the per-test Setup, but no additional groups in between? Not quite. This test is to make sure that we only run the top-level test. What I really should be doing is adding to `WebInspectorAudit.__test` so that we can confirm that `"A"` never gets added to the string. The idea is that only top-level `setup` are ever run.

Created attachment 363856 [details] Patch

Comment on attachment 363856 [details] Patch Clearing flags on attachment: 363856 Committed r242808: <https://trac.webkit.org/changeset/242808>

All reviewed patches have been landed. Closing bug.