Bug 196583 - TestWebKitAPI config.h should be aware of what suite is being built
Summary: TestWebKitAPI config.h should be aware of what suite is being built
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-03 17:22 PDT by Don Olmstead
Modified: 2019-05-02 15:33 PDT (History)
8 users (show)

See Also:

Attachments
WIP Patch (44.48 KB, patch)
2019-04-29 13:03 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (44.92 KB, patch)
2019-04-29 13:06 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (45.42 KB, patch)
2019-04-29 14:06 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (47.75 KB, patch)
2019-04-29 14:27 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (48.67 KB, patch)
2019-04-29 14:50 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (48.90 KB, patch)
2019-04-29 14:59 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (48.93 KB, patch)
2019-04-29 15:17 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (49.00 KB, patch)
2019-04-29 15:26 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (50.33 KB, patch)
2019-04-29 15:51 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (50.00 KB, patch)
2019-04-29 15:54 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (50.02 KB, patch)
2019-04-29 16:13 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (50.31 KB, patch)
2019-04-29 16:22 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (56.07 KB, patch)
2019-04-29 16:47 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (56.73 KB, patch)
2019-04-30 09:19 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (57.68 KB, patch)
2019-04-30 13:57 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (57.83 KB, patch)
2019-04-30 14:43 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (58.03 KB, patch)
2019-04-30 16:40 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
WIP Patch (58.23 KB, patch)
2019-04-30 18:28 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
Patch (59.94 KB, patch)
2019-05-01 09:46 PDT, Don Olmstead 		annulen: review+
Details | Formatted Diff | Diff
Patch (59.94 KB, patch)
2019-05-01 16:07 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
Show Obsolete (19) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 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.
Comment 1 Don Olmstead 2019-04-29 13:03:06 PDT Comment hidden (obsolete)
Comment 2 EWS Watchlist 2019-04-29 13:06:06 PDT Comment hidden (obsolete)
Comment 3 Don Olmstead 2019-04-29 13:06:16 PDT Comment hidden (obsolete)
Comment 4 EWS Watchlist 2019-04-29 13:08:18 PDT Comment hidden (obsolete)
Comment 5 Don Olmstead 2019-04-29 14:06:34 PDT Comment hidden (obsolete)
Comment 6 EWS Watchlist 2019-04-29 14:08:27 PDT Comment hidden (obsolete)
Comment 7 Don Olmstead 2019-04-29 14:27:35 PDT Comment hidden (obsolete)
Comment 8 EWS Watchlist 2019-04-29 14:29:53 PDT Comment hidden (obsolete)
Comment 9 Don Olmstead 2019-04-29 14:50:01 PDT Comment hidden (obsolete)
Comment 10 EWS Watchlist 2019-04-29 14:51:47 PDT Comment hidden (obsolete)
Comment 11 Don Olmstead 2019-04-29 14:59:49 PDT Comment hidden (obsolete)
Comment 12 EWS Watchlist 2019-04-29 15:00:55 PDT Comment hidden (obsolete)
Comment 13 Don Olmstead 2019-04-29 15:17:33 PDT Comment hidden (obsolete)
Comment 14 EWS Watchlist 2019-04-29 15:20:17 PDT Comment hidden (obsolete)
Comment 15 Don Olmstead 2019-04-29 15:26:01 PDT Comment hidden (obsolete)
Comment 16 EWS Watchlist 2019-04-29 15:27:57 PDT Comment hidden (obsolete)
Comment 17 Don Olmstead 2019-04-29 15:51:24 PDT Comment hidden (obsolete)
Comment 18 Don Olmstead 2019-04-29 15:54:29 PDT Comment hidden (obsolete)
Comment 19 EWS Watchlist 2019-04-29 15:57:41 PDT Comment hidden (obsolete)
Comment 20 Don Olmstead 2019-04-29 16:13:35 PDT Comment hidden (obsolete)
Comment 21 EWS Watchlist 2019-04-29 16:15:56 PDT Comment hidden (obsolete)
Comment 22 Don Olmstead 2019-04-29 16:22:27 PDT Comment hidden (obsolete)
Comment 23 EWS Watchlist 2019-04-29 16:25:54 PDT Comment hidden (obsolete)
Comment 24 Don Olmstead 2019-04-29 16:47:57 PDT Comment hidden (obsolete)
Comment 25 EWS Watchlist 2019-04-29 16:49:39 PDT Comment hidden (obsolete)
Comment 26 Don Olmstead 2019-04-30 09:19:16 PDT Comment hidden (obsolete)
Comment 27 EWS Watchlist 2019-04-30 09:23:25 PDT Comment hidden (obsolete)
Comment 28 Konstantin Tokarev 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()
Comment 29 Don Olmstead 2019-04-30 13:57:38 PDT Comment hidden (obsolete)
Comment 30 EWS Watchlist 2019-04-30 14:00:07 PDT Comment hidden (obsolete)
Comment 31 Don Olmstead 2019-04-30 14:43:57 PDT Comment hidden (obsolete)
Comment 32 EWS Watchlist 2019-04-30 14:46:06 PDT Comment hidden (obsolete)
Comment 33 Don Olmstead 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.
Comment 34 Konstantin Tokarev 2019-04-30 15:23:10 PDT 
Ok
Comment 35 Don Olmstead 2019-04-30 16:40:24 PDT Comment hidden (obsolete)
Comment 36 EWS Watchlist 2019-04-30 16:43:43 PDT Comment hidden (obsolete)
Comment 37 Don Olmstead 2019-04-30 18:28:04 PDT 
Created attachment 368641 [details]
WIP Patch

Think all bots will be happy after this.
Comment 38 EWS Watchlist 2019-04-30 18:30:37 PDT Comment hidden (obsolete)
Comment 39 Don Olmstead 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?
Comment 40 Michael Catanzaro 2019-05-01 07:22:43 PDT 
Beware today is a holiday.

I would just land it and watch https://build.webkit.org/dashboard/
Comment 41 Don Olmstead 2019-05-01 09:46:46 PDT 
Created attachment 368679 [details]
Patch

Think this is ready for a review.
Comment 42 EWS Watchlist 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.
Comment 43 Konstantin Tokarev 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?
Comment 44 Don Olmstead 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.
Comment 45 Konstantin Tokarev 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
Comment 46 Don Olmstead 2019-05-01 16:07:57 PDT 
Created attachment 368726 [details]
Patch
Comment 47 WebKit Commit Bot 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>
Comment 48 WebKit Commit Bot 2019-05-01 16:47:36 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 49 Radar WebKit Bug Importer 2019-05-02 15:33:38 PDT 
<rdar://problem/50424785>