Instead of compiling each series of tests into a separate binary, we can greatly simplify the API test compilation by having WTF tests in one binary and WebKit2 tests in another.
Created attachment 146156 [details] Patch
Comment on attachment 146156 [details] Patch I noticed some issues with the new makefile. I'll fix and upload a new patch.
Created attachment 146394 [details] Patch
Comment on attachment 146394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146394&action=review I think the patch breaks the option --skipped=ignore > Tools/ChangeLog:19 > + * gtk/run-api-tests: Change the way that tests are skipped by splitting out the > + concept of skipping a test and skipping a suite (program) of tests. Test cases are > + skipped because of legitimate failures, but entire programs are skipped because of > + problems in the harness. As of right now a test program is only skipped if the > + accessibility bus cannot be started. I think I prefer the simple approach of having only one way of handling skipped tests, the SkippedTest class already contains the program name after all. If the skipped tests doesn't contain a test case, the whole program will be skipped. > Tools/ChangeLog:21 > + (SkippedTest.__init__): Make the test case a required argument and have one skipped > + test case per SkippedTest instance. This makes a lot of sense, since different test cases in the same suite can fail for different reasons. > Tools/TestWebKitAPI/GNUmakefile.am:56 > + Tools/TestWebKitAPI/gtk/main.cpp \ > + Tools/TestWebKitAPI/Test.h \ > + Tools/TestWebKitAPI/TestsController.cpp \ > + Tools/TestWebKitAPI/TestsController.h \ These are common and part of libTestWebKitAPIMain
Comment on attachment 146394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146394&action=review >> Tools/ChangeLog:19 >> + accessibility bus cannot be started. > > I think I prefer the simple approach of having only one way of handling skipped tests, the SkippedTest class already contains the program name after all. If the skipped tests doesn't contain a test case, the whole program will be skipped. So perhaps the SkippedTest class could have a constructor argument that says whether or not the test is affected by the --skipped argument. I think it's still useful to skip the a11y test when the bus fails to start (regardless of the command-line arguments). >> Tools/TestWebKitAPI/GNUmakefile.am:56 >> + Tools/TestWebKitAPI/TestsController.h \ > > These are common and part of libTestWebKitAPIMain Nice catch. I'll remove them.
(In reply to comment #4) > I think the patch breaks the option --skipped=ignore Oh, missed this! How does it break --skipped=ignore?
(In reply to comment #5) > (From update of attachment 146394 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146394&action=review > > >> Tools/ChangeLog:19 > >> + accessibility bus cannot be started. > > > > I think I prefer the simple approach of having only one way of handling skipped tests, the SkippedTest class already contains the program name after all. If the skipped tests doesn't contain a test case, the whole program will be skipped. > > So perhaps the SkippedTest class could have a constructor argument that says whether or not the test is affected by the --skipped argument. I think it's still useful to skip the a11y test when the bus fails to start (regardless of the command-line arguments). You don't need a constructor argument, if the skipped test doesn't have any test case the whole program is skipped. Yes, I agree we should skip the program if the a11y bus fails to start regardless of the --skipped. > >> Tools/TestWebKitAPI/GNUmakefile.am:56 > >> + Tools/TestWebKitAPI/TestsController.h \ > > > > These are common and part of libTestWebKitAPIMain > > Nice catch. I'll remove them.
(In reply to comment #6) > (In reply to comment #4) > > > I think the patch breaks the option --skipped=ignore > > Oh, missed this! How does it break --skipped=ignore? Because you are not readin that option anymore, ignore means runs the tests even if they are skipped (useful to detect skipped tests that now pass), except for cases where we want to skip the whole program like the a11y bus not started.
Created attachment 147911 [details] Patch
(In reply to comment #8) > Because you are not readin that option anymore, ignore means runs the tests even if they are skipped (useful to detect skipped tests that now pass), except for cases where we want to skip the whole program like the a11y bus not started. --ignore=skip was in fact working, but --ignore=only has been broken for a while. Carlos agreed that we can fix that in a followup patch. I've addressed his other concerns in this new patch.
Comment on attachment 147911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147911&action=review Looks good to me, please fix the a11y skipped test before landing. > Tools/gtk/run-api-tests:160 > + self._skipped_tests.append(SkippedTest(WebKit2APITests/TestWebKitAccessibility, SkippedTest.ENTIRE_SUITE, "Could not start accessibility bus", 0)) I think this line is longer than 120, it would be easier to read if it's split, I think. What is the last 0? the bug number? You don't need to give that parameter as it takes None by default and we explicitly check for None. Does it even work? the test name is missing quotation marks
Committed r120944: <http://trac.webkit.org/changeset/120944>