Bug 57992

See bug 57987 for the changes that needed to happen for NRWT.

Created attachment 95524 [details] Patch

Comment on attachment 95524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95524&action=review > Tools/Scripts/old-run-webkit-tests:2305 > + } elsif ($lineIn =~ /(.*)#EOF$/) { > + if ($1) { This will do the wrong thing if $1 is "0". Perl silently converts that to a number, which then becomes false. You could just compare to "". > Tools/Scripts/old-run-webkit-tests:2336 > + my $joined_error = join("", @error); Nit: Pulling this out into a variable doesn't seem to add any value.

Comment on attachment 95524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95524&action=review >> Tools/Scripts/old-run-webkit-tests:2305 >> + if ($1) { > > This will do the wrong thing if $1 is "0". Perl silently converts that to a number, which then becomes false. You could just compare to "". Good catch. Will fix. >> Tools/Scripts/old-run-webkit-tests:2336 >> + my $joined_error = join("", @error); > > Nit: Pulling this out into a variable doesn't seem to add any value. I thought this made the return slightly easier to read. I'll revert it.

Created attachment 95656 [details] update w/ review feedback from Tony

Comment on attachment 95656 [details] update w/ review feedback from Tony Clearing flags on attachment: 95656 Committed r87873: <http://trac.webkit.org/changeset/87873>

All reviewed patches have been landed. Closing bug.

I have no idea how, but this patch broke two tests on Qt port: --- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/events/overflow-viewport-renderer-deleted-expected.txt 2011-06-02 07:01:35.955265937 -0700 +++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/events/overflow-viewport-renderer-deleted-actual.txt 2011-06-02 07:01:35.955265937 -0700 @@ -0,0 +1 @@ +ERROR: nil result from [documentElement innerText] \ No newline at end of file --- /ramdisk/qt-linux-64-release/build/layout-test-results/plugins/document-open-expected.txt 2011-06-02 07:04:46.240269339 -0700 +++ /ramdisk/qt-linux-64-release/build/layout-test-results/plugins/document-open-actual.txt 2011-06-02 07:04:46.240269339 -0700 @@ -1 +1,2 @@ CONSOLE MESSAGE: line 0: PLUGIN: DOCUMENT OPEN SUCCESS +ERROR: nil result from [documentElement innerText] \ No newline at end of file

.

(In reply to comment #8) > I have no idea how, but this patch broke two tests on Qt port: > > --- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/events/overflow-viewport-renderer-deleted-expected.txt 2011-06-02 07:01:35.955265937 -0700 > +++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/events/overflow-viewport-renderer-deleted-actual.txt 2011-06-02 07:01:35.955265937 -0700 > @@ -0,0 +1 @@ > +ERROR: nil result from [documentElement innerText] > \ No newline at end of file > > > --- /ramdisk/qt-linux-64-release/build/layout-test-results/plugins/document-open-expected.txt 2011-06-02 07:04:46.240269339 -0700 > +++ /ramdisk/qt-linux-64-release/build/layout-test-results/plugins/document-open-actual.txt 2011-06-02 07:04:46.240269339 -0700 > @@ -1 +1,2 @@ > CONSOLE MESSAGE: line 0: PLUGIN: DOCUMENT OPEN SUCCESS > +ERROR: nil result from [documentElement innerText] > \ No newline at end of file The patch changed things so that if you had text on the final line of output from DRT but no trailing newline, then that text would be included in the -actual output (previously it would have been dropped), e.g.: foo\n bar#EOF would produce an actual.txt of foo\n bar^D whereas previously the bar would've been dropped. If your DRT was somehow not appending a final newline for some tests, this would break that. The fix would be to update the baselines to contain the newly appended text and (optionally) fix DRT, since it should always be putting in a final newline for text files. (This change was needed because there is no final newline for audio files at the moment).

I think we can (and should) close this again; Ossy, do you agree?

(In reply to comment #11) > I think we can (and should) close this again; Ossy, do you agree? Yes. Thanks for the information. I updated Qt specific results: http://trac.webkit.org/changeset/87998. Gabor, could you check this EOF/newline problem? As far as I remember you fixed a similar bug in DRT before.