Bug 57992 – old-run-webkit-tests: add support for audio files

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dirk Pranke]

See bug 57969 - we're adding support to DRT to output audio files to test web audio, and we need to add support to ORWT to handle the output correctly.

[Dirk Pranke]

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

[Dirk Pranke]

Created attachment 95524 [details]
Patch

[Tony Chang]

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.

[Dirk Pranke]

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.

[Dirk Pranke]

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

[WebKit Commit Bot]

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

Clearing flags on attachment: 95656

Committed r87873: <http://trac.webkit.org/changeset/87873>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

.

[Dirk Pranke]

(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).

[Dirk Pranke]

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

[Csaba Osztrogonác]

(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.