Bug 144828 – [GTK] REGRESSION(r183936): Test /webkit2/WebKitWebContext/spell-checker fails since r183936

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

Developer mode now sets a language by default.

[Martin Robinson]

Created attachment 252778 [details]
Patch

[Martin Robinson]

Created attachment 252779 [details]
Patch

[Carlos Garcia Campos]

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.

[Martin Robinson]

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.

[Carlos Garcia Campos]

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

[Martin Robinson]

Created attachment 255822 [details]
Patch

[Carlos Garcia Campos]

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

[Martin Robinson]

Committed r186175: <http://trac.webkit.org/changeset/186175>