Currently torch-launcher only run tests parsed by NRWT, with ref-tests excluded. As a result, if a test have ref-tests (which were not run), our DumpRenderTree Perl script will think there's a crash and exit with code 1. So NRWT will report the result as CRASH. We need to update DumpRenderTree.cpp to try to find ref-tests and run them. Example: When running test "css2.1/20110323/border-conflict-element-001.htm" we should be able to find its ref-test "css2.1/20110323/border-conflict-element-001-expected.html" and run it as well.
Created attachment 141090 [details] Git patch for bug 86055
Attachment 141090 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Tools/DumpRenderTree/blackbe..." exit_code: 1 Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:236: An else should appear on the same line as the preceding } [whitespace/newline] [4] Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:248: Missing space before ( in if( [whitespace/parens] [5] Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:259: An else should appear on the same line as the preceding } [whitespace/newline] [4] Total errors found: 3 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 141094 [details] Git patch updated for code style
Comment on attachment 141094 [details] Git patch updated for code style View in context: https://bugs.webkit.org/attachment.cgi?id=141094&action=review Looks good, can be cleaned up some more. > ChangeLog:1 > +2012-05-09 Xiaobo Wang <xbwang@torchmobile.com.cn> You need an empty line here. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:206 > + const char* surfixes[] = {"-expected", "-expected-mismatch"}; Typo, suffix. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:207 > + const int countofSurfixes = sizeof(surfixes) / sizeof(const char*); Ditto. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:406 > + for (Vector<WTF::String>::iterator iter = files.begin(); files.end() != iter; ++iter) Can you try if it works if you do not add WTF:: in this cpp file?
Xiaobo, aren't you going to rename this? + int getRefTests(const WTF::String& testName); Also, the return value never gets checked.
Created attachment 141333 [details] Git patch updated for comments from Bob and Ming
Thanks Rob, please forgive my poor spelling and carelessness:) Without WTF:: it also works, so I cleaned it for all instances in DumpRenderTree.cpp and .h.
(In reply to comment #5) > Xiaobo, aren't you going to rename this? > + int getRefTests(const WTF::String& testName); > > Also, the return value never gets checked. Thanks Ming. updated to return void.
(In reply to comment #6) > Created an attachment (id=141333) [details] > Git patch updated for comments from Bob and Ming Sorry, I mean Rob not Bob.
Comment on attachment 141333 [details] Git patch updated for comments from Bob and Ming View in context: https://bugs.webkit.org/attachment.cgi?id=141333&action=review Looks much better! Please fix remaining issues before landing. > ChangeLog:8 > + Ref-tests are tests with surfix "-expected", "-expected-mismatch" and a valid surfix! > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:209 > + const char* extensions[] = {".html"}; We may want to add .svg as possible ref-test extension later. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:222 > + Remove empty line. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:401 > + // and multiple mismatch ref-tests Lacks a period at the end. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:403 > + getRefTests(*iter); I think it should be getRefTest, not getRefTests.
(In reply to comment #9) > (In reply to comment #6) > > Created an attachment (id=141333) [details] [details] > > Git patch updated for comments from Bob and Ming > > Sorry, I mean Rob not Bob. No problem :)
(In reply to comment #10) Again thanks for the good catches! > > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:209 > > + const char* extensions[] = {".html"}; > > We may want to add .svg as possible ref-test extension later. Great, I'll add ".svg"to the array. > > > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:403 > > + getRefTests(*iter); > > I think it should be getRefTest, not getRefTests. Why do you think so? A test may have multiple ref-tests.
Created attachment 141382 [details] Git patch updated for more comments from Rob
Comment on attachment 141382 [details] Git patch updated for more comments from Rob Looks good.
Comment on attachment 141382 [details] Git patch updated for more comments from Rob Clearing flags on attachment: 141382 Committed r116758: <http://trac.webkit.org/changeset/116758>
Closing, this landed in r116758.
Comment on attachment 141333 [details] Git patch updated for comments from Bob and Ming Clear r/cq flags for obsolete patch.
Clear r/cq flags for obsolete patch.
Comment on attachment 141333 [details] Git patch updated for comments from Bob and Ming Clear.
close.