Investigate how we might get rid of the Quick testing framework

[lawrence-forooghian]

What?

I'd like us to consider getting rid of the Quick testing framework.

Why?

It's a non-standard thing for new developers to learn, but more importantly it's actually making the test suite harder to maintain.

I was recently trying to investigate some tests that are intermittently failing in CI and on my machine. I wanted to see what happens when I try running these tests in isolation instead of in the full suite. Unfortunately, this wasn’t really possible because of Quick’s lack of integration with the Xcode testing UI controls. So, I think removing it would help us towards our short-term goal of identifying and quarantining flaky tests.

How?

This would be a very time-consuming thing to do manually – our test suite is big. I think it would be possible to automate it, though, using some sort of code transformation tool such as SwiftSyntax with some simple rules, to transform our QuickSpec subclasses into XCTestCase ones.

I think automating it, other than saving us a heap of time, would save us from having to have disruptive code freezes whilst we migrate a file. It would also allow us to experiment with different approaches for how we’d like the migrated code to look.

I suggest that we prototype something to see whether this automation is viable.

References

• Internal Slack conversation

┆Issue is synchronized with this Jira Uncategorised by Unito

[lawrence-forooghian]

A progress update, and some further thoughts.

What I've done

I've put together a prototype using SwiftSyntax, which is capable of:

• changing the superclass of a QuickSpec object into XCTestCase
• transforming all it invocations contained directly inside (i.e. without an intermediate context or describe) a spec() declaration into top-level func test* instance methods

Neither of these are hugely complicated things but they've given me confidence in how to use SwiftSyntax to interrogate the contents of a method and extract things like calls to context(), describe() etc, and how to generate new code.

Viability of the approach

I believe that we will be able to extend this prototype to handle the (more complicated) nested test cases. This is where having access to the AST will truly show its utility over a simple find-and-replace.

Potential complications for automation

We have some (I don't know how many) dynamically-defined tests — for example, it being called in a loop with different parameters. There probably isn't going to be anything automated we can do with these, and my suggestion is that as we encounter them, we switch them to being individual it calls which call a shared method. We can do this as a sequence of small groundwork PRs. As I say, I'm not sure how much work this will be because I think there's no easy way to find them by hand, so we'll find them programatically as we start delving deeper into the syntax tree.

Considerations for landing this change

We need to make sure that these changes are reviewable by a human being. That means we need to be able to view a diff that shows only the structural changes to the tests (we can't expect anyone to review our ~25,000 lines of test code, and even less so expect them to compare two different sets of 25,000 lines of code to make sure most of it hasn't changed by accident). That means that we'll probably need to preserve the current indentation levels of the tests, even though syntactically we're completely flattening the structure (since all of the generated func test* methods will now be at the top level of their class). Once the changes have been reviewed and merged, we can do a second pass with a code formatter to sort out the indentation.

Other logistical considerations

• We have a bunch of GitHub issues that relate to failing test cases. We need to make sure that it's still possible to locate the test case referred to by these issues. Not thought much about that yet.

Decisions to make

• We have to figure out exactly how we want to flatten the test structure (i.e. how we want to combine the describe / context (possibly multiple) / it names into a single test method). We can experiment with different options here. I wish there were some way to not have to flatten the test structure – that is, to find some alternative syntactical structure that allows us to preserve the nesting. (It would also probably make the whole exercise amenable to a find & replace instead.) I couldn't think of anything, though. I thought about nested XCTestCase subclasses (or even a hierarchy of subclasses, but that brings further problems like duplicated tests) but this is no use since Xcode doesn't know what to do with the results – you end up with a list of mangled class names in the Xcode test navigator after running the tests.

• Figure out how we'd like to handle tests skipped using xit – func skip_test method name, or something else? (Look into XCSkip)

[lawrence-forooghian]

I think the next steps, if we're happy to continue down the automation route, are to build out the prototype to handle nesting.

[lawrence-forooghian]

A status update. Once I finished the first version of the migrator, the migrated code had three new test failures. Long story short, I've spent a fair chunk of time investigating these, operating under the assumption that they must result from the behaviour of the migrated code differing from that of the Quick code. I believe I've now got the migrator emitting code that behaves almost identically to the original Quick code, and I'm happy to say this appears to have resolved the failures.

I'm now going to start landing some of this work into main, starting with some groundwork PRs.