[test] Split TestFoundation in smaller tests for CTest reporting.

[drodriguez]

At the moment, only one test was created for CTest, which would have run
all the test in TestFoundation, and would have reported the final result
as the test result, but without details of which test have failed.

Following the similar case of GTest and gtest_discover_tests, XCTest can
be used to list all the tests in the suite, and execute them as invidual
CTest. They will report as different tests in the output, and individual
failures will be shown. It will be also a little bit more resilient
against crashes, which will only affect one test, and not the full
suite.

At first I was thinking in doing the parsing in some scripting language,
but in order to be platform independent, and because the parsing is easy
enough, the parsing of the XCTest output is done in CMake.

Tested on Linux building Foundation for Linux. I will have to check if the code works on Windows. Cross-compiling and running the tests would be complicated, so I'm not sure about that scenario either. Improvements welcome.

[spevans]

Will this work correctly for the tests wrapped in shouldRunXFailTests which are enabled by the
NS_FOUNDATION_ATTEMPT_XFAIL_TESTS environment variable?

[drodriguez]

@spevans: the test names are listed by XCTest, so they will be part of the test suite. If the environment variable is not set, though, the test body is just an empty block, so they will be reported as "Passed". If the variable is set, the test body will be the normal.

[gmittert]

This looks good to me and would be really helpful to have. Each individual test gets passed in the same environment that the single test run does so it'll respect NS_FOUNDATION_ATTEMPT_XFAIL_TESTS.

[gmittert]

@swift-ci please test

[drodriguez]

@swift-ci please test

[compnerd]

This is horrible. Such is CMake.

[gmittert]

@swift-ci please test

[spevans]

Interesting that the test that failed (TestURLSession.test_setCookies) only works if test_cookiesStorage is run before it, obviously its setting up some state. This test also fails when run on its own against Darwin Foundation. I think it should be disabled for now.

[drodriguez]

Yes, from a fast skim of the test, it seems that the test is checking if the Cookie: header is sent from the client to the server, by making the server reply with the request headers in the response body. The test doesn't make it easy to figure out the body is the one that contains the "header-like" content.

I will rewrite those tests a little bit and send another PR. This PR might be better to merge after @compnerd merges the one about 3.15 anyway.

[gmittert]

@swift-ci please test

[drodriguez]

Fixed that suffix/prefix problem.

@swift-ci please test

[drodriguez]

@swift-ci please test Linux platform

[drodriguez]

@compnerd: I know we talked about waiting for 3.15 support to land. Should we stick to that plan?

@gmittert: I think this should work on Windows. Can you confirm if you have time? Thanks.

[drodriguez]

Rebased on top of a current master and rewritten for the changes introduced by CMake 3.15. Let's see if this works and we can merge it.

@swift-ci please test Linux platform

[spevans]

@drodriguez Just thought of a couple of questions re this PR:

1. Is there a timeout applied to each invoked test? I believe there is currently a 30 minute one applied by Jenkins to catch hanging tests, is it possible to apply say a 3min timeout to each test?
2. Can the tests be run in parallel?

[drodriguez]

1. At the moment there was no timeout in the original, so there's no timeout after the changes. I don't see where that 30 minutes timeout is setup (private Jenkins configuration?) so I don't know if it will apply to each test or to all the tests. We can probably set a short timeout for each test (2 or 3 minutes?), if we want.
2. I would not recommend running the test in parallel, since they weren't running in parallel and some of them might interact with some others in funny ways. Tests that use some shared storage (for example the cookies that I fixed in the related PR), will step into each other. One can probably run subsets of the tests in different machines to be safe. But answering your question directly, setting CTEST_PARALLEL_LEVEL should execute them in parallel.

[drodriguez]

@spevans, @millenomi: some other concern about this change. I would like to have a LGTM from someone else before going forward with this. Thanks!

[spevans]

@drodriguez I think its worth merging this, my only concern would be an increased runtime of all of the tests since TestFoundation will now be fork()/exec()'d for each test but we can always revert if it shows a big increase (or even group the tests by test class)

[drodriguez]

@spevans: I cannot deny the increased runtime is there (I don't see if I left it written somewhere, but I remember that in my machine it was adding around 1 extra minute in a full run). Personally, however, I consider a fair price to pay to have independent tests and more reliability. Grouping the tests by class will improve a little the time, and should avoid most of the problems, but I would rather not do it unless there's a strong opinion for it.

I will try to find some time to rebase and recheck these changes, and merge then. Thanks for looking into them.

[spevans]

@drodriguez Thats fine, we just need to keep an eye on it post merge so the CI folks dont complain.

[drodriguez]

From 58 seconds to 167 seconds in a beefy machine. Not sure if the extra time is just all the printing, or the fork/exec dance, or if its constant or depends on the number of tests. In exchange, we have test by test reporting of success/failure and timing. If someone feels it is too much time, we can revert and try another approach.

[drodriguez]

@swift-ci please test Linux platform

[spevans]

@swift-ci please test Linux platform

[drodriguez]

In the CI machines:

In a previous PR without this changes:

Total Test time (real) =  57.62 sec

With the changes:

Total Test time (real) = 179.39 sec

More or less the same timings I got in my machine.

As I wrote before. We can revert easily if somebody feels it is too much time and little benefit.

[compnerd]

I think that this broke the Windows builds at least: https://dev.azure.com/compnerd/swift-build/_build/results?buildId=24334&view=logs&j=231dcdbf-b818-57d0-e791-37f9a41be191&t=8799e978-1854-5f0c-ec22-c3511b5c45aa

[drodriguez]

Looks like a missing DLL from the error code. I think because the PATH change happens after rebuilding, but with this change, the test executable needs to execute once during compilation.

I would recommend move the Update PATH step before Re-Configure Foundation instead of before Test Foundation.

[compnerd]

Running with that change here: https://dev.azure.com/compnerd/swift-build/_build/results?buildId=24343&view=results ... lets see if that fixes it

[compnerd]

Thanks, that does fix it

[drodriguez]

Seems that you might need to run ctest -T test manually to get the XML, though. I don’t know if it is possible to do it from cmake --target test.