Refactor FXIOS-7301 - Remove 1 closure_body_length violation from RustAutofillTests.swift

[ionixjunior]

📜 Tickets

Jira ticket
Github issue

💡 Description

This PR removes 1 closure_body_length violation from the RustAutofillTests.swift file by extracting the creation of the UnencryptedCreditCardFields object to a new function.

This PR is part of a series of PRs described here #16442 (comment).

📝 Checklist

You have to check all boxes before merging

• Filled in the above information (tickets numbers and description of your work)
• Updated the PR name to follow our PR naming guidelines
• Wrote unit tests and/or ensured the tests suite is passing
• When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
• If needed, I updated documentation / comments for complex code and public methods
• If needed, added a backport comment (example @Mergifyio backport release/v120)

[FilippoZazzeroni]

Looks good to me, i would only refactor the force cast with a conditional cast, since there is already the plan to get this pattern removed. Let me know what you think. Thanks again 🎉

[ionixjunior]

This makes sense. I've made the updates and used guard let for unwrapping, which works well here. I chose this approach because I found three variables in this test that were using force unwrap: creditCard, card, and updatedCardVal.

I liked this approach because it avoids excessive indentation and keeps the code readable. I also noticed similar force unwrapping in other tests methods but haven't updated those yet.

What do you think of this adjustment?

[FilippoZazzeroni]

I like it with guards, but in the test area there is a better thing.
I would do:

let card = try XCTUnwrap(card)

So whit this approach you can remove all the guards and the XCTAssertNotNil since they are incorporated in the XCTUnwrap

[ionixjunior]

Interesting! I noticed some examples in the project that use XCTUnwrap, but they're applied in standard functions rather than closures. When I tried to apply it to the creditCard variable here, I encountered this error:

Invalid conversion from throwing function of type '(CreditCard?, (any Error)?) throws -> Void' to non-throwing function type '(CreditCard?, (any Error)?) -> Void'

The same applies to the card variable. To make this work, I would need to refactor the code further to ensure the closere is throwable, but I'm unsure if this is feasible. Another approach would be to use a do catch block with XCTUnwrap, though this would introduce additional indentation, similar to the if let approach.

What do you think would be the best way to proceed?

[FilippoZazzeroni]

I think with the guard let we are loosing the information when a test is failing, i'd go with the do catch so we can fail the test in the catch block by passing the error message, what do you think ?

[ionixjunior]

I've implemented it, but this increases the number of lines due to the three new do catch blocks. To address this, I've extracted the final assertions into separated functions to keep the main closure under 30 lines and ensures the compliance with the closure_body_length rule.

This might not be the best solution, but without extracting the final assertions, the main closure would still be around 40 lines.

Let me know your thoughts on this approach.