Summary: | [GTK] REGRESSION(r183936): Test /webkit2/WebKitWebContext/spell-checker fails since r183936 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||||
Component: | WebKitGTK | Assignee: | Martin Robinson <mrobinson> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cgarcia, mcatanzaro | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 144648 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Martin Robinson
2015-05-09 09:43:06 PDT
Created attachment 252778 [details]
Patch
Created attachment 252779 [details]
Patch
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 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. (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. Created attachment 255822 [details]
Patch
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 Committed r186175: <http://trac.webkit.org/changeset/186175> |