AztecEditor-iOS icon indicating copy to clipboard operation
AztecEditor-iOS copied to clipboard

Use Xcode 15.3 in CI

Open mokagio opened this issue 1 year ago • 5 comments

Builds on top of https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1391, where I noticed that CI didn't run because we no longer have agents with Xcode 13 online.

To test: See green CI running on Xcode 15.3 image... via #1393 which marks four tests as expected failures 😞


  • [x] I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

mokagio avatar Apr 16 '24 04:04 mokagio

The test that locks is testCopyAndPasteToPlainText from TextViewTests. It didn't occur for me locally till I tried to run the test class individually.

mokagio avatar Apr 16 '24 06:04 mokagio

The test that locks is testCopyAndPasteToPlainText from TextViewTests. It didn't occur for me locally till I tried to run the test class individually.

Figured out why...

Screenshot 2024-04-16 at 19 49 34

mokagio avatar Apr 16 '24 09:04 mokagio

Ah! I also realized that @jkmassel had already worked out that issue in #1377. I had seen some code about UIPasteboard .forTesting and was surprised not to find it on my local. I then realized this branch was out of sync with develop.

mokagio avatar Apr 16 '24 10:04 mokagio

@mokagio I took the liberty to push a commit (https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1392/commits/665b15bd56ad4a0331c13f71045d693613486362) to fix the testPaste{Image,Video}WithoutFormatting test failures.

[EDIT]I later fixed that commit 665b15b with c67821c[/EDIT]


  • I have searched the code for decodeObject(forKey: occurrences and confirmed that I've replaced all of them with decodeObject(of: …, forKey: …) instead
  • But I have not triple-checked that every custom subclass of anything we might encode in our NSAttributedString attributes have had +supportsSecureCoding redefined

Indeed, apparently, even if the parent class already overrides it to return true, subclasses which override init?(coder:)/encode(with:) from their parent class need to also re-override +supportsSecureCoding.

I've added the overrides in classes that I've modified, but haven't check if maybe there are more classes that might not be covered by our unit tests around NSAttributedString archiving/pasting, but would still require it to be added. So would be worth making another pass to be sure we didn't forget any (otherwise the client app would start dismissing those attributes during pasting, while it didn't before our migration to iOS12).

AliSoftware avatar Apr 18 '24 11:04 AliSoftware

@mokagio Just also pushed https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1392/commits/90c0e4db1246dc4e87f004c4ec96061399447efb to fix the testCopyAndPasteToPlainText test failure on extra \n.

⚠️ That being said, while the test pass, I'm still not 100% sure why there was a \n when we were using public.plain-text before, so maybe there's more to it and it could still be worth investigating further (and add more tests to cover the cases we missed if needed)

There's also still another test failing, this time in WordPressEditor where testCaptionShortcodeIsProperlyConvertedIntoFigureTagPreservingNestedTags() seems to have an extra space in the output. I haven't looked into that one so I'll leave it to you to take a look.

AliSoftware avatar Apr 18 '24 13:04 AliSoftware