Refactoring after fixing test cases discoverage in Xcode 13.3 #1134
Conversation
…es into new class - AllExamplesBuilder
…of AllExamplesBuilder (and flag 'didBuildAllExamples') depends on the lifetime of world
… it's not needed
…d class) into #if !SWIFTP_PACKAGE directive because this way of initialization is used only in ObjC version of QuickSpec which is not for SPM
Generated by 🚫 Danger |
|
Danger fails as usual because I'm not a contributor. Ran it locally - it's ok (see comment above). |
There was a problem hiding this comment.
Thanks @BobCatC ! 💯
I think this looks good.
One request:
Can you comment to call out what was simply moved vs what was changed? I think I understand what's happening, but this would be very helpful — and good for posterity.
|
@BobCatC oh also, I've marked this for the v5.0 release. This would be the final PR to merge for that. It doesn't seem very risky, but I would appreciate your thoughts on the risk level as well. |
| } | ||
|
|
||
| // swiftlint:disable:next todo | ||
| // TODO: Unify this with QuickConfiguration's equivalent |
There was a problem hiding this comment.
I think we should address this before merging.
But I don't have a ton of context here.
|
Hey @younata -- not sure if you saw, but GitHub restored all the previously-deleted Russian account contributions. So this PR is back now. 😊 https://www.jessesquires.com/blog/2022/04/19/github-suspending-russian-accounts/#updated-21-april-2022 |
Intro
Next step after #1129
As we agreed with @jessesquires this is a refactoring PR after small fix of Xcode 13.3 issue. What code was refactored read below.
AllExamplesBuilderclassJust extracted logic of calling method
buildExamplesIfNeededon eachQuickSpecsubclass fromQuickTestObservationinto separate class.You can note that the instance has a flag -
didBuildAllExampleswhich helps to execute method only once per lifetime of the object. And also you can note that the object is used as a property ofWorldinstance. So its lifetime equals world's lifetime (which in real projects acts like a singleton).Tests for the issue
I've added a test for Xcode 13.3 issue. And to be able to test that behaviour I had to add into function
qck_runSpecnew param -gatherExamples. Passingfalsesimulates Xcode 13.3 problem, passingtrue(which is default) behaves like before.Checklist