Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get KIF CI working reliably with Xcode 14.3.1 & 15.4 #1300

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

justinseanmartin
Copy link
Contributor

@justinseanmartin justinseanmartin commented Aug 8, 2024

I'd recommend reviewing this commit-by-commit, as they're logically organized and have verbose commit messages. Some notable things:

KIF behavior changes:

PullToRefresh test flakiness:
This was probably the trickiest thing that I discovered while digging in here. There were a bunch of flakes happening on CI that I couldn't really explain. What I saw when running these tests locally was the pull to refresh control would be dismissed abruptly after the action completed, despite it being configured to wait for up to 4s to complete. What I found was that the behavior in #1020 to instantly scroll without animations to enumerate table view cells is what is causing the abrupt cancellation of the pull to refresh operation. This was unexpected and I think a side-effect of the behavior change in #1020.

Luckily, in #1296, @SimoncelloCT added the capability to prevent scrolling the view when enumerating the view hierarchy. I leveraged the functionality there to stabilize these tests.

Other test flakiness:
There were a bunch of small changes required to get the tests to pass more reliably. I tried reproducing a lot of these with slow animations enabled locally in the simulator settings. CI is working much better now with this change.

Misc cleanup:

  • I got rid of Carthage references that weren't being validated. I'm not sure if it still works or not, but we weren't validating it, so I'm just reflecting that state of the world.
  • The KIFFramework target (and respective tests) target wasn't compiling and I couldn't find an easy way to fix that and not break SPM at the same time. For the TestableSwift and Calculator example projects, had them consume KIF via SPM instead.
  • I got rid of spaces from the folders and reorganized the files in the file system to make it cleaner. The top level folder now looks like this:
drwxr-xr-x   Documentation
drwxr-xr-x   KIF Tests
-rw-r--r--   KIF.podspec
drwxr-xr-x@  KIF.xcodeproj
-rw-r--r--@  LICENSE
-rw-r--r--   Package.swift
-rw-r--r--   README.md
drwxr-xr-x   Scripts
drwxr-xr-x   Sources
drwxr-xr-x   TestHost
drwxr-xr-x   Tests

Configuring CI to run in 4 configurations:
 - xcode 14.3.1, iPad Air (5th generation), iOS 16.4
 - xcode 14.3.1, iPhone 14, iOS 16.4
 - xcode 15.4, iPad Air (5th generation), iOS 17.5
 - xcode 15.4, iPhone 15, iOS 17.5

As part of this change, the CI script was updated to only run the
extra validation of the examples folder one one of the 4 shards.

This removes Carthage from the README as being officially supported.
I'd guess that is generally not being used. The CI script hasn't worked
since Xcode 12.
* AccessibilityIdentifierTests tests

Fix `testEnteringTextIntoViewWithAccessibilityIdentifier` flaking

We're trying to tap on `Select All` while it is still being animated into view. The problem is two-fold:

1. The `longPress` action doesn't wait for animations to complete, it just uses a hardcoded .5s before
starting the next action.
2. Even after switching to use `waitForAnimationsToFinish`, the timeout defaults to .5s as the max to
wait for animations to complete.

To fix this, I switched `longPress` action to use `waitForAnimationsToFinish` and I upped the
default timeout to wait up to 2s for animations to complete. This seems to work much more reliably now.

* SystemTests.testMockingOpenURL

NSURL no longer parses '423543523454://', use a different protocol that starts with characters

* ExistTests

Wait a bit longer for the view controller presentation to complete

* TypingTests

In TapViewController, avoid race condition in resigning first responder and becoming first responder.
CI was flaking with a failure where "Deleted something., world" was found in the greeting text field.

* PullToRefreshTests

Leverage 'usingCurrentFrame' so we're not scrolling the table view around
while the UIRefreshControl is spinning, which immediately halts the refresh
spinner. Also, make sure that we're scrolled to the top of the test suite VC,
as otherwise the pull to refresh action might fail.
It doesn't really make sense that the default animation wait time
is the same as the stabilization time. This effectively means the
behavior of scanning the UI hierarchy for animations in progress is
never utilized for most tests out of the box.

For what its worth, we've used a 2s animation wait time and .1s
stabilization time as our defaults for a very long time.

This should generally not slow down any tests if there weren't any
animations in progress. If there were animations in progress, this
is probably a worthwhile tradeoff to make things more reliable. This
can still be configured globally to be overidden.

As part of this change, use `waitForAnimationsToFinish` in place of
a static .5s sleep for the `longPress` action.

There will be a followup PR kif-framework#1301 that should speed up some scenarios
where we're waiting longer than desired for animations to complete.
Have the pullToRefresh action pull from 25% of the way down the element to 75% of the way down, instead of pulling from the center and pulling a full height of the element
This removes the `KIFFramework` target and its consumer tests. They don't
interop well with the way that angle and quote imports work in SPM. More info:

https://joesusnick.medium.com/swift-package-manager-with-a-mixed-swift-and-objective-c-project-part-2-2-e71dad234e6

Took this opportunity to remove the spaces in the target names for the 'Testable Swift'
target, in case that was causing issues somewhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant