[GTK] TestInspectorServer unit test is timing out

[Zan Dobersek]

Both on release and debug builds. Will skip it.

[Anton Obzhirov]

Going check this one. Probably started to fail after GTK inspector server has been re-factored to Soup inspector server.

[Anton Obzhirov]

It does time out in debug mode - managed to reproduce it. Will debug tomorrow

[Anton Obzhirov]

Well, found out the problem finally - webinspector resources are not installed in default folder. Going to fix it now.

[Anton Obzhirov]

Created attachment 206654 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Gustavo Noronha (kov)]

Comment on attachment 206654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206654&action=review

r- because we should move copying of files to the build system

> Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:48
> +    // Overwrite WEBKIT_INSPECTOR_SERVER_PATH variable to point to inspector resources folder.
> +    const gchar* inspectorResourcesPath = g_getenv("WEBKIT_INSPECTOR_PATH");    
> +    g_setenv("WEBKIT_INSPECTOR_SERVER_PATH", inspectorResourcesPath, TRUE);

It makes sense to have this variable being set here, but when you say overwrite, do you mean the variable is already set but set to the wrong path?

> Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:55
> +    // Copy inspectorPageIndex.html to inspector resources folder
> +    GOwnPtr<char> sourcePath(g_build_filename(WEBKIT_SRC_DIR, "Source", "WebKit2", "UIProcess", "InspectorServer", "front-end", "inspectorPageIndex.html", NULL));
> +    GRefPtr<GFile> source = adoptGRef(g_file_new_for_path(sourcePath.get()));
> +    GOwnPtr<char> destinationPath(g_build_filename(inspectorResourcesPath, "inspectorPageIndex.html", NULL));
> +    GRefPtr<GFile> destination = adoptGRef(g_file_new_for_path(destinationPath.get()));
> +    g_file_copy(source.get(), destination.get(), G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL); 

We should do this in the build system like we do for that other bits, instead.

[Anton Obzhirov]

(In reply to comment #6)
> (From update of attachment 206654 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206654&action=review
> 
> r- because we should move copying of files to the build system
> 
> > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:48
> > +    // Overwrite WEBKIT_INSPECTOR_SERVER_PATH variable to point to inspector resources folder.
> > +    const gchar* inspectorResourcesPath = g_getenv("WEBKIT_INSPECTOR_PATH");    
> > +    g_setenv("WEBKIT_INSPECTOR_SERVER_PATH", inspectorResourcesPath, TRUE);
> 
> It makes sense to have this variable being set here, but when you say overwrite, do you mean the variable is already set but set to the wrong path?
No, it shouldn’t be set. But just in case if there is "leftover" after another test. It feels safer for me to overwrite it.
> 
> > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:55
> > +    // Copy inspectorPageIndex.html to inspector resources folder
> > +    GOwnPtr<char> sourcePath(g_build_filename(WEBKIT_SRC_DIR, "Source", "WebKit2", "UIProcess", "InspectorServer", "front-end", "inspectorPageIndex.html", NULL));
> > +    GRefPtr<GFile> source = adoptGRef(g_file_new_for_path(sourcePath.get()));
> > +    GOwnPtr<char> destinationPath(g_build_filename(inspectorResourcesPath, "inspectorPageIndex.html", NULL));
> > +    GRefPtr<GFile> destination = adoptGRef(g_file_new_for_path(destinationPath.get()));
> > +    g_file_copy(source.get(), destination.get(), G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL); 
> 
> We should do this in the build system like we do for that other bits, instead.
Yep, I will update GNUmakefile.am for WebKit2 to copy the file.

[Carlos Garcia Campos]

run-gtk-tests sets WEBKIT_INSPECTOR_PATH

[Anton Obzhirov]

(In reply to comment #8)
> run-gtk-tests sets WEBKIT_INSPECTOR_PATH
Yes, and then I use it to set WEBKIT_INSPECTOR_SERVER_PATH inside the test. Or you mean I should set WEBKIT_INSPECTOR_SERVER_PATH in run-gtk-tests as well?

[Carlos Garcia Campos]

(In reply to comment #9)
> (In reply to comment #8)
> > run-gtk-tests sets WEBKIT_INSPECTOR_PATH
> Yes, and then I use it to set WEBKIT_INSPECTOR_SERVER_PATH inside the test. Or you mean I should set WEBKIT_INSPECTOR_SERVER_PATH in run-gtk-tests as well?

Ah, SERVER_PATH, I see, sorry, I read too fast. I think it's fine to set it in the test then.

[Anton Obzhirov]

Created attachment 206787 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 206787 [details]
Patch

OK

[WebKit Commit Bot]

Comment on attachment 206787 [details]
Patch

Clearing flags on attachment: 206787

Committed r153085: <http://trac.webkit.org/changeset/153085>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.