Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add subscriber network connectivity tests
Resolves #539. These are based on the corresponding tests in the Android codebase at commit f6d163f. They stick as closely as possible to the structure and behaviour of those tests, to which I have not applied much of a critical eye. The majority of the log messages and comments are simply copied across. The only structural difference between these tests and the Android ones is that I have removed the caller’s responsibility to call `SubscriberMonitor#close`. There are places in the Android implementation where we forget to call it, and also it isn’t called if an error is thrown by `waitForStateTransition`. By making the subscriber monitor responsible for its own cleanup, we remove these issues. When I started writing these tests, we were still targeting iOS 12 and above, which meant it was not possible to use Swift concurrency. Hence, all of the code is written using either completion handlers or blocking functions. Since then, we’ve decided to set our deployment target to iOS 13 (see #597), which means we could now use Swift concurrency. However, by that point I was well into the writing of these tests and did not want to re-structure them, especially since they require functionality that doesn’t exist out of the box in Swift concurrency (for example timeouts). We may wish to revisit at some point and switch to using Swift concurrency, but I don’t think it’s urgent. In order to generate the list of which faults to skip, I ran all of the tests and checked which ones failed. Whilst doing so, I encountered a crash in ably-cocoa, which I believe is already reported as ably/ably-cocoa#1380. In order to get past this issue, I tried using Marat’s fix from ably-cocoa branch `fix/1380-dispatch-ARTOSReachability_Callback` (commit 036be28). This removed the crashes. The list of skipped tests is the list of tests that failed when using this branch. It then actually turned out that the non-skipped tests do not exhibit this crash anyway, so I didn’t include any version change to ably-cocoa in this PR. We can deal with this crash when we try re-enabling the tests in #575, by which point hopefully the fix for ably-cocoa#1380 will have been released. (Internal thread re this crash and its appearance in these tests is [1].) [1] https://ably-real-time.slack.com/archives/CSQEKCE81/p1678802535684519
- Loading branch information