[webkit-reviews] review denied: [Bug 59816] [Refactoring] Replace Node's Document pointer with a TreeScope pointer : [Attachment 179174] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 12 20:01:52 PST 2012


Ryosuke Niwa <rniwa at webkit.org> has denied Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 59816: [Refactoring] Replace Node's Document pointer with a TreeScope
pointer
https://bugs.webkit.org/show_bug.cgi?id=59816

Attachment 179174: Patch
https://bugs.webkit.org/attachment.cgi?id=179174&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
(In reply to comment #89)
> (In reply to comment #88)
> > (In reply to comment #87)
> > > Did you run all the tests in Debug mode?
> > 
> > I didn't request a commit because I haven't run all the tests yet, I did
run fast/mutation and fast/dom though and this fixes it. Right now I can't get
it to build because r137539 broke the chromium build, so I'll run them later
tonight/tomorrow and update.
> 
> run-webkit-tests --chromium --debug --no-pixel-tests	LayoutTests/fast/
> 
> 9502 tests executed and I didn't see any crashes.

For the change of this nature that could affect literally everything in DOM,
it’s not acceptable to run just a subset of tests.

WebKit contribution policy that contributors run all layout tests prior to
landing the patch: http://www.webkit.org/quality/testing.html
"Before patches can land in any of the frameworks in the repository, the layout
regression tests must pass. To run these tests, execute the run-webkit-tests
script."

In practice, you may get away with it by running a subset if you know that the
patch only affects a subset of tests in advance but we can’t necessarily do
that all the time.

Please run the entire layout test suite and report the result (I typically
start run-webkit-tests before I go to lunch or go home).


More information about the webkit-reviews mailing list