Bug 36227 - [GTK] Failing tests http/tests/misc/image-blocked-src-change.html & http/tests/misc/image-blocked-src-no-change.html
Summary: [GTK] Failing tests http/tests/misc/image-blocked-src-change.html & http/test...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-17 10:21 PDT by Sergio Villar Senin
Modified: 2010-03-18 06:32 PDT (History)
3 users (show)

See Also:


Attachments
Patch that modifies the way we print local URIs in DRT (3.88 KB, patch)
2010-03-17 10:35 PDT, Sergio Villar Senin
xan.lopez: review-
Details | Formatted Diff | Diff
Patch that modifies the way we print local URIs in DRT (5.29 KB, patch)
2010-03-18 03:46 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2010-03-17 10:21:24 PDT
Both tests are failing because we are printing full URLs while tests only expect filenames
Comment 1 Sergio Villar Senin 2010-03-17 10:35:35 PDT
Created attachment 50922 [details]
Patch that modifies the way we print local URIs in DRT

Tests expect only the file name for the local URIs. This patch makes DRT print the local URIs as expected by tests
Comment 2 Xan Lopez 2010-03-17 12:08:50 PDT
Comment on attachment 50922 [details]
Patch that modifies the way we print local URIs in DRT

>+    gchar* testMessage = NULL;

Let's try to use 0 instead of NULL, since it's what the style guide says (yes, I realize this file in particular has a mix of NULLs and 0s...)

>+    const gchar* uriScheme;
>+
>+    // Tests expect only the filename part of local URIs
>+    uriScheme = g_strstr_len(message, -1, "file://");
>+    if (uriScheme) {
>+        GString* tempString = g_string_sized_new(strlen(message));
>+        gchar* filename = g_strrstr(uriScheme, G_DIR_SEPARATOR_S);
>+
>+        if (filename) {
>+            filename++;
>+            tempString = g_string_append_len(tempString, message, (uriScheme - message));
>+            tempString = g_string_append_len(tempString, filename, strlen(filename));
>+            testMessage = g_string_free(tempString, FALSE);
>+        }

You are assuming that there's only one file:// and that it will be the last thing in the string, right? The first assumption seems to be shared by all DRTs, but not the second one, so perhaps you should find the last separator going forward from file:// ? You could just jump to the first whitespace and then either do it manually or use g_path_get_basename, I guess.

>+    }
>+
>+    fprintf(stdout, "CONSOLE MESSAGE: line %d: %s\n", line, (testMessage) ? testMessage : message);

The parenthesis around testMessage are not really needed.

>+    g_free(testMessage);
>+
>     return TRUE;
> }
> 
>-- 
>1.7.0
>
Comment 3 Sergio Villar Senin 2010-03-18 03:12:53 PDT
(In reply to comment #2)
> (From update of attachment 50922 [details])
> >+    const gchar* uriScheme;
> >+
> >+    // Tests expect only the filename part of local URIs
> >+    uriScheme = g_strstr_len(message, -1, "file://");
> >+    if (uriScheme) {
> >+        GString* tempString = g_string_sized_new(strlen(message));
> >+        gchar* filename = g_strrstr(uriScheme, G_DIR_SEPARATOR_S);
> >+
> >+        if (filename) {
> >+            filename++;
> >+            tempString = g_string_append_len(tempString, message, (uriScheme - message));
> >+            tempString = g_string_append_len(tempString, filename, strlen(filename));
> >+            testMessage = g_string_free(tempString, FALSE);
> >+        }
> 
> You are assuming that there's only one file:// and that it will be the last
> thing in the string, right? The first assumption seems to be shared by all
> DRTs, but not the second one, so perhaps you should find the last separator
> going forward from file:// ? You could just jump to the first whitespace and
> then either do it manually or use g_path_get_basename, I guess.

I'm indeed assuming that there is only one "file://" but nothing more IMHO. In 

tempString = g_string_append_len(tempString, filename, strlen(filename));

I'm appending everything starting from the filename, so if there is anything more after the filename it will be copied as strlen(filename) will not stop until the end of the "message" string is found. Or maybe I'm not getting your point...
Comment 4 Sergio Villar Senin 2010-03-18 03:46:17 PDT
Created attachment 51017 [details]
Patch that modifies the way we print local URIs in DRT

New version of the patch with Xan's requests

I have also removed another 7 extra tests from the Skipped file that were failing for the same reason
Comment 5 Xan Lopez 2010-03-18 03:57:40 PDT
Comment on attachment 51017 [details]
Patch that modifies the way we print local URIs in DRT

r=me
Comment 6 WebKit Commit Bot 2010-03-18 06:32:06 PDT
Comment on attachment 51017 [details]
Patch that modifies the way we print local URIs in DRT

Clearing flags on attachment: 51017

Committed r56157: <http://trac.webkit.org/changeset/56157>
Comment 7 WebKit Commit Bot 2010-03-18 06:32:10 PDT
All reviewed patches have been landed.  Closing bug.