Skip to content

AINFRA-781 - Make YosemiteTests a Swift package test target #15797

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

AINFRA-781 - Make YosemiteTests a Swift package test target #15797

merged 1 commit into from
Jun 26, 2025

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 22, 2025 •
edited
Loading

Description

Now that all the frameworks sources are Swift packages (targets in the same Swift package to be precise) we can move the tests there, too. We didn't do that all in one go to avoid big diffs.

Notice:

  • The whole Yosemite/ folder has been removed, including the dedicated Xcode project
  • No changes in the test code
  • WooCommerceTests references a few types from YosemiteTests. I was not able to do without them. I'm sure it's possible by creating a tests helper package, but it felt out of scope at this time.

Testing information

See green CI.

Additionally, you can run the focused Hardware tests by selecting the Hardware scheme from Xcode.

Screenshots

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. — N.A.

@dangermattic
Copy link
Collaborator

dangermattic commented Jun 22, 2025 •
edited
Loading

2 Warnings
⚠️ Modules/Package.swift was changed without updating its corresponding Package.resolved. Please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running swift package resolve).
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@mokagio mokagio added this to the 22.8 milestone Jun 22, 2025
@mokagio mokagio self-assigned this Jun 22, 2025
@mokagio mokagio added category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. Testing labels Jun 22, 2025
@mokagio mokagio changed the title AINFRA-778 - Make StorageTests a Swift package test target AINFRA-781 - Make YosemiteTests a Swift package test target Jun 22, 2025
@mokagio mokagio changed the base branch from ainfra-780-move-hardwaretests-to-modules to ainfra-779-move-networkingtests-to-modules June 22, 2025 23:59
mokagio
mokagio commented Jun 23, 2025
View reviewed changes
Modules/Package.swift
],
resources: [
.process("Resources"),
.process("../NetworkingTests/Responses")
Copy link
Contributor Author

@mokagio mokagio Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also #15796 description regarding sharing the Responses assets

@mokagio mokagio force-pushed the ainfra-781-move-yosemitetests-to-modules branch from 8275c2a to 3577fb5 Compare June 23, 2025 00:03
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 23, 2025 •
edited
Loading

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Number30748
VersionPR #15797
Bundle IDcom.automattic.alpha.woocommerce
Commit3b8bb65
Installation URL584mrrletc4u8
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@mokagio mokagio requested review from a team June 23, 2025 00:21
@mokagio mokagio force-pushed the ainfra-779-move-networkingtests-to-modules branch from 915a045 to 550c2eb Compare June 24, 2025 00:07
itsmeichigo
itsmeichigo reviewed Jun 24, 2025
View reviewed changes
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the same issues that Jaclyn reported in the previous PR: the old projects still exist in the navigator and I have to switch schemes to run tests for a different target:

image

I suppose you'd need to force push again to bring fixes from the previous PR to this one. I'm also seeing conflicts, looks like the tests were still being updated while this work is in progress. I hope it's straightforward to handle the conflicts.

Regarding the testing information section:

Additionally, you can run the focused Hardware tests by selecting the Hardware scheme from Xcode.

I confirm that CI is passing, but I don't understand why I need to run Hardware tests in this PR. I tried running Yosemite tests and got a crash:

freed pointer was not the last allocation

It's odd that this happened locally and not on CI. I hope it's an issue with the selected simulator, like how Gabriel encountered the crash in StorageTests in #15794.

@mokagio mokagio force-pushed the ainfra-779-move-networkingtests-to-modules branch from 550c2eb to 93ec55a Compare June 24, 2025 20:38
@mokagio
Copy link
Contributor Author

mokagio commented Jun 25, 2025

Hi @itsmeichigo . Sorry for the late reply. I wanted to ack your comments. I haven't addressed them yet because I'll wait for the base PR to land, #15796 . Thanks!

Base automatically changed from ainfra-779-move-networkingtests-to-modules to trunk June 25, 2025 12:06
@mokagio mokagio force-pushed the ainfra-781-move-yosemitetests-to-modules branch 2 times, most recently from 38cdac3 to 35d3684 Compare June 26, 2025 00:54
@mokagio mokagio requested a review from itsmeichigo June 26, 2025 01:36
@mokagio
Copy link
Contributor Author

mokagio commented Jun 26, 2025

Here we go @itsmeichigo everything is green now. Would appreciate a new review 🙇‍♂️

@mokagio mokagio enabled auto-merge June 26, 2025 01:38
@mokagio mokagio force-pushed the ainfra-781-move-yosemitetests-to-modules branch from 35d3684 to 3b8bb65 Compare June 26, 2025 01:39
itsmeichigo
itsmeichigo approved these changes Jun 26, 2025
View reviewed changes
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and confirmed that the issues in my last comment were resolved. I ran Yosemite tests and they passed without issues.

@mokagio mokagio merged commit 79a5ab1 into trunk Jun 26, 2025
13 checks passed
@mokagio mokagio deleted the ainfra-781-move-yosemitetests-to-modules branch June 26, 2025 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@itsmeichigo itsmeichigo itsmeichigo approved these changes

Assignees

@mokagio mokagio

Labels

category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. Testing

Projects

None yet

Milestone

22.8

Development

Successfully merging this pull request may close these issues.

5 participants

@mokagio @dangermattic @wpmobilebot @itsmeichigo