Bug 132686 - REGRESSION(CMAKE): [GTK] InputMethodFilter unit test is not built
Summary: REGRESSION(CMAKE): [GTK] InputMethodFilter unit test is not built
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Regression
Depends on: 132685
Blocks: 132739
  Show dependency treegraph
 
Reported: 2014-05-08 09:14 PDT by Carlos Garcia Campos
Modified: 2014-05-14 05:49 PDT (History)
7 users (show)

See Also:


Attachments
Patch (32.39 KB, patch)
2014-05-09 05:53 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (32.31 KB, patch)
2014-05-10 23:40 PDT, Carlos Garcia Campos
pnormand: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2014-05-08 09:14:12 PDT
Since the switch to cmake we are not building the InputMethodFilter unit test, I think this was part of the TestWebCore binary that doesn't exist anymore either.
Comment 1 Carlos Garcia Campos 2014-05-09 05:53:59 PDT
Created attachment 231148 [details]
Patch

Bring it back and move it to a single binary as well.
Comment 2 Martin Robinson 2014-05-09 09:45:02 PDT
Comment on attachment 231148 [details]
Patch

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

> Tools/ChangeLog:9
> +        a single binary TestWebCore with the other Webcore tests. Also

Super nit: Webcore -> WebCore

> Tools/TestWebKitAPI/PlatformGTK.cmake:125
> -set(test_webcore_BINARIES
> -    LayoutUnit
> -    URL
> +add_executable(TestWebCore
> +    ${test_main_SOURCES}
> +    ${TestWebCoreGtk_SOURCES}
> +    ${TESTWEBKITAPI_DIR}/TestsController.cpp
> +    ${TESTWEBKITAPI_DIR}/Tests/WebCore/LayoutUnit.cpp
> +    ${TESTWEBKITAPI_DIR}/Tests/WebCore/URL.cpp
>  )

I think if we are going to go our own way on all of these unit tests, we should just stop using the shared CMakeLists.txt.

> Tools/TestWebKitAPI/PlatformGTK.cmake:130
> +add_test(TestWebCore ${TESTWEBKITAPI_RUNTIME_OUTPUT_DIRECTORY}/WebCore/TestWebCore)
> +set_tests_properties(TestWebCore PROPERTIES TIMEOUT 60)
> +set_target_properties(TestWebCore PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${TESTWEBKITAPI_RUNTIME_OUTPUT_DIRECTORY}/WebCore)

We don't need these lines unless we plan to use 'make test.'
Comment 3 Carlos Garcia Campos 2014-05-09 23:43:30 PDT
(In reply to comment #2)
> (From update of attachment 231148 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231148&action=review
> 
> > Tools/ChangeLog:9
> > +        a single binary TestWebCore with the other Webcore tests. Also
> 
> Super nit: Webcore -> WebCore

:-)

> > Tools/TestWebKitAPI/PlatformGTK.cmake:125
> > -set(test_webcore_BINARIES
> > -    LayoutUnit
> > -    URL
> > +add_executable(TestWebCore
> > +    ${test_main_SOURCES}
> > +    ${TestWebCoreGtk_SOURCES}
> > +    ${TESTWEBKITAPI_DIR}/TestsController.cpp
> > +    ${TESTWEBKITAPI_DIR}/Tests/WebCore/LayoutUnit.cpp
> > +    ${TESTWEBKITAPI_DIR}/Tests/WebCore/URL.cpp
> >  )
> 
> I think if we are going to go our own way on all of these unit tests, we should just stop using the shared CMakeLists.txt.

No, WTF tests and the injected bundle lib for WebKit2 tests are shared in CMakeLists.txt, I would propose the other way around and move this approach to the common file, but that would be a different issue, because we were already adding those binaries in the platform specific file.
Comment 4 Carlos Garcia Campos 2014-05-10 23:40:01 PDT
Created attachment 231254 [details]
Rebased patch
Comment 5 Carlos Garcia Campos 2014-05-14 05:49:21 PDT
Committed r168834: <http://trac.webkit.org/changeset/168834>