RESOLVED FIXED Bug 196583
TestWebKitAPI config.h should be aware of what suite is being built
https://bugs.webkit.org/show_bug.cgi?id=196583
Summary TestWebKitAPI config.h should be aware of what suite is being built
Don Olmstead
Reported 2019-04-03 17:22:56 PDT
Currently all tests have a dependency on each library's export macros due to includes in the config.h. Proposing we add -DBUILDING_TEST<Target> to the compile options to cut down on what is being built.
Attachments
WIP Patch (44.48 KB, patch)
2019-04-29 13:03 PDT, Don Olmstead
no flags
WIP Patch (44.92 KB, patch)
2019-04-29 13:06 PDT, Don Olmstead
no flags
WIP Patch (45.42 KB, patch)
2019-04-29 14:06 PDT, Don Olmstead
no flags
WIP Patch (47.75 KB, patch)
2019-04-29 14:27 PDT, Don Olmstead
no flags
WIP Patch (48.67 KB, patch)
2019-04-29 14:50 PDT, Don Olmstead
no flags
WIP Patch (48.90 KB, patch)
2019-04-29 14:59 PDT, Don Olmstead
no flags
WIP Patch (48.93 KB, patch)
2019-04-29 15:17 PDT, Don Olmstead
no flags
WIP Patch (49.00 KB, patch)
2019-04-29 15:26 PDT, Don Olmstead
no flags
WIP Patch (50.33 KB, patch)
2019-04-29 15:51 PDT, Don Olmstead
no flags
WIP Patch (50.00 KB, patch)
2019-04-29 15:54 PDT, Don Olmstead
no flags
WIP Patch (50.02 KB, patch)
2019-04-29 16:13 PDT, Don Olmstead
no flags
WIP Patch (50.31 KB, patch)
2019-04-29 16:22 PDT, Don Olmstead
no flags
WIP Patch (56.07 KB, patch)
2019-04-29 16:47 PDT, Don Olmstead
no flags
WIP Patch (56.73 KB, patch)
2019-04-30 09:19 PDT, Don Olmstead
no flags
WIP Patch (57.68 KB, patch)
2019-04-30 13:57 PDT, Don Olmstead
no flags
WIP Patch (57.83 KB, patch)
2019-04-30 14:43 PDT, Don Olmstead
no flags
WIP Patch (58.03 KB, patch)
2019-04-30 16:40 PDT, Don Olmstead
no flags
WIP Patch (58.23 KB, patch)
2019-04-30 18:28 PDT, Don Olmstead
no flags
Patch (59.94 KB, patch)
2019-05-01 09:46 PDT, Don Olmstead
annulen: review+
Patch (59.94 KB, patch)
2019-05-01 16:07 PDT, Don Olmstead
no flags
Don Olmstead
Comment 1 2019-04-29 13:03:06 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 2 2019-04-29 13:06:06 PDT Comment hidden (obsolete)
Don Olmstead
Comment 3 2019-04-29 13:06:16 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 4 2019-04-29 13:08:18 PDT Comment hidden (obsolete)
Don Olmstead
Comment 5 2019-04-29 14:06:34 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 6 2019-04-29 14:08:27 PDT Comment hidden (obsolete)
Don Olmstead
Comment 7 2019-04-29 14:27:35 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 8 2019-04-29 14:29:53 PDT Comment hidden (obsolete)
Don Olmstead
Comment 9 2019-04-29 14:50:01 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 10 2019-04-29 14:51:47 PDT Comment hidden (obsolete)
Don Olmstead
Comment 11 2019-04-29 14:59:49 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 12 2019-04-29 15:00:55 PDT Comment hidden (obsolete)
Don Olmstead
Comment 13 2019-04-29 15:17:33 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 14 2019-04-29 15:20:17 PDT Comment hidden (obsolete)
Don Olmstead
Comment 15 2019-04-29 15:26:01 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 16 2019-04-29 15:27:57 PDT Comment hidden (obsolete)
Don Olmstead
Comment 17 2019-04-29 15:51:24 PDT Comment hidden (obsolete)
Don Olmstead
Comment 18 2019-04-29 15:54:29 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 19 2019-04-29 15:57:41 PDT Comment hidden (obsolete)
Don Olmstead
Comment 20 2019-04-29 16:13:35 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 21 2019-04-29 16:15:56 PDT Comment hidden (obsolete)
Don Olmstead
Comment 22 2019-04-29 16:22:27 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 23 2019-04-29 16:25:54 PDT Comment hidden (obsolete)
Don Olmstead
Comment 24 2019-04-29 16:47:57 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 25 2019-04-29 16:49:39 PDT Comment hidden (obsolete)
Don Olmstead
Comment 26 2019-04-30 09:19:16 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 27 2019-04-30 09:23:25 PDT Comment hidden (obsolete)
Konstantin Tokarev
Comment 28 2019-04-30 12:20:13 PDT
Comment on attachment 368572 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368572&action=review > Tools/TestWebKitAPI/CMakeLists.txt:4 > +add_definitions(${gtest_DEFINITIONS}) We should not use add_definitions(). Instead, gtest target should have PUBLIC (or INTERFACE) definitions, which will be used automatically for any target where gtest is linked > Tools/TestWebKitAPI/PlatformWin.cmake:10 > +add_definitions(-DSTATICALLY_LINKED_WITH_PAL=1) Use target_compile_definitions() (or COMPILE_DEFINITIONS property) instead of deprecated add_definitions()
Don Olmstead
Comment 29 2019-04-30 13:57:38 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 30 2019-04-30 14:00:07 PDT Comment hidden (obsolete)
Don Olmstead
Comment 31 2019-04-30 14:43:57 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 32 2019-04-30 14:46:06 PDT Comment hidden (obsolete)
Don Olmstead
Comment 33 2019-04-30 15:21:24 PDT
(In reply to Konstantin Tokarev from comment #28) > Comment on attachment 368572 [details] > WIP Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=368572&action=review > > > Tools/TestWebKitAPI/CMakeLists.txt:4 > > +add_definitions(${gtest_DEFINITIONS}) > > We should not use add_definitions(). Instead, gtest target should have > PUBLIC (or INTERFACE) definitions, which will be used automatically for any > target where gtest is linked I feel that should be a separate task after this lands.
Konstantin Tokarev
Comment 34 2019-04-30 15:23:10 PDT
Ok
Don Olmstead
Comment 35 2019-04-30 16:40:24 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 36 2019-04-30 16:43:43 PDT Comment hidden (obsolete)
Don Olmstead
Comment 37 2019-04-30 18:28:04 PDT
Created attachment 368641 [details] WIP Patch Think all bots will be happy after this.
EWS Watchlist
Comment 38 2019-04-30 18:30:37 PDT Comment hidden (obsolete)
Don Olmstead
Comment 39 2019-04-30 18:35:38 PDT
Adding some Igalia folks since to make sure that we aren't breaking their test runs. Build should be working now but I haven't run tests on anything. Can you folks make sure that the tests are running as expected?
Michael Catanzaro
Comment 40 2019-05-01 07:22:43 PDT
Beware today is a holiday. I would just land it and watch https://build.webkit.org/dashboard/
Don Olmstead
Comment 41 2019-05-01 09:46:46 PDT
Created attachment 368679 [details] Patch Think this is ready for a review.
EWS Watchlist
Comment 42 2019-05-01 09:48:58 PDT
Attachment 368679 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/config.h:50: "JavaScriptCore/JSExportMacros.h" already included at Tools/TestWebKitAPI/config.h:46 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:56: "JavaScriptCore/JSExportMacros.h" already included at Tools/TestWebKitAPI/config.h:46 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:57: "WebCore/PlatformExportMacros.h" already included at Tools/TestWebKitAPI/config.h:51 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:58: "pal/ExportMacros.h" already included at Tools/TestWebKitAPI/config.h:52 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:58: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/TestWebKitAPI/config.h:65: "JavaScriptCore/JSExportMacros.h" already included at Tools/TestWebKitAPI/config.h:46 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:66: "WebCore/PlatformExportMacros.h" already included at Tools/TestWebKitAPI/config.h:51 [build/include] [4] ERROR: Tools/TestWebKitAPI/config.h:67: "pal/ExportMacros.h" already included at Tools/TestWebKitAPI/config.h:52 [build/include] [4] Total errors found: 8 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Konstantin Tokarev
Comment 43 2019-05-01 14:49:32 PDT
Comment on attachment 368679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368679&action=review > Tools/TestWebKitAPI/PlatformGTK.cmake:110 > + ${FORWARDING_HEADERS_DIR} Looks like something we wanted to get rid off, doesn't it?
Don Olmstead
Comment 44 2019-05-01 14:53:03 PDT
(In reply to Konstantin Tokarev from comment #43) > Comment on attachment 368679 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=368679&action=review > > > Tools/TestWebKitAPI/PlatformGTK.cmake:110 > > + ${FORWARDING_HEADERS_DIR} > > Looks like something we wanted to get rid off, doesn't it? Windows is copying all its headers around. GTK and WPE still have the forwarding headers being generated for WebKit and some other testing stuff. I just want to leave this alone for the moment until I get WebKit fully copying headers.
Konstantin Tokarev
Comment 45 2019-05-01 14:59:19 PDT
Comment on attachment 368679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368679&action=review LGTM except that letter case error > Tools/TestWebKitAPI/PlatformWin.cmake:98 > +set(TestWebcore_OUTPUT_NAME TestWebCore${DEBUG_SUFFIX}) Wrong letter case in TestWebcore_OUTPUT_NAME
Don Olmstead
Comment 46 2019-05-01 16:07:57 PDT
WebKit Commit Bot
Comment 47 2019-05-01 16:47:34 PDT
Comment on attachment 368726 [details] Patch Clearing flags on attachment: 368726 Committed r244857: <https://trac.webkit.org/changeset/244857>
WebKit Commit Bot
Comment 48 2019-05-01 16:47:36 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 49 2019-05-02 15:33:38 PDT
Note You need to log in before you can comment on or make changes to this bug.