Bug 144828 - [GTK] REGRESSION(r183936): Test /webkit2/WebKitWebContext/spell-checker fails since r183936
Summary: [GTK] REGRESSION(r183936): Test /webkit2/WebKitWebContext/spell-checker fails...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on: 144648
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-09 09:43 PDT by Martin Robinson
Modified: 2015-07-02 11:59 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2015-05-09 09:48 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (2.00 KB, patch)
2015-05-09 10:16 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (8.52 KB, patch)
2015-06-30 09:23 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2015-05-09 09:43:06 PDT
Developer mode now sets a language by default.
Comment 1 Martin Robinson 2015-05-09 09:48:57 PDT
Created attachment 252778 [details]
Patch
Comment 2 Martin Robinson 2015-05-09 10:16:14 PDT
Created attachment 252779 [details]
Patch
Comment 3 Carlos Garcia Campos 2015-05-10 01:02:03 PDT
Comment on attachment 252779 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:247
> +#if ENABLE_DEVELOPER_MODE

This is always true here. We don't support tests in production builds, because they use internal symbols. This is a problem, because this is supposed to test the behaviour of the API in a production build, of course.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:248
> +    // Developer mode builds have a language preset, for the purposes of layout tests.

Would it be possible to add the default language from WTR instead of doing that unconditionally for development builds? Maybe adding new internals API.
Comment 4 Martin Robinson 2015-05-10 18:04:50 PDT
Comment on attachment 252779 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:247
>> +#if ENABLE_DEVELOPER_MODE
> 
> This is always true here. We don't support tests in production builds, because they use internal symbols. This is a problem, because this is supposed to test the behaviour of the API in a production build, of course.

Good point. The tests should really be runnable for production builds.

>> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:248
>> +    // Developer mode builds have a language preset, for the purposes of layout tests.
> 
> Would it be possible to add the default language from WTR instead of doing that unconditionally for development builds? Maybe adding new internals API.

Yeah, this might be a decent moment to remove the hack I added for spell checking. I could add some static C API to set the languages.
Comment 5 Carlos Garcia Campos 2015-05-10 23:57:57 PDT
(In reply to comment #4)
> Comment on attachment 252779 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252779&action=review
> 
> >> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:247
> >> +#if ENABLE_DEVELOPER_MODE
> > 
> > This is always true here. We don't support tests in production builds, because they use internal symbols. This is a problem, because this is supposed to test the behaviour of the API in a production build, of course.
> 
> Good point. The tests should really be runnable for production builds.

We decided to make it easier to write the tests and use things like WTF, for example, which makes impossible to run the tests in production builds where all those symbols are not available. However, we should try to ensure that the behaviour of what we test is the same in a production build.

> >> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:248
> >> +    // Developer mode builds have a language preset, for the purposes of layout tests.
> > 
> > Would it be possible to add the default language from WTR instead of doing that unconditionally for development builds? Maybe adding new internals API.
> 
> Yeah, this might be a decent moment to remove the hack I added for spell
> checking. I could add some static C API to set the languages.

Exactly.
Comment 6 Martin Robinson 2015-06-30 09:23:14 PDT
Created attachment 255822 [details]
Patch
Comment 7 Carlos Garcia Campos 2015-06-30 22:40:54 PDT
Comment on attachment 255822 [details]
Patch

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

Thank you!

> Tools/WebKitTestRunner/gtk/main.cpp:39
> +    g_ptr_array_add(languages.get(), 0);

nullptr
Comment 8 Martin Robinson 2015-07-01 07:52:26 PDT
Committed r186175: <http://trac.webkit.org/changeset/186175>