Skip to content
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.

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

Add support for swift-testing based tests #1313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ed-irl
Copy link

@ed-irl ed-irl commented Sep 25, 2024

This adds support for the new swift-testing library.

Presently, test suites that contain swift-testing will output additional test information in addition to the standard XCTest output, which is always written. You can see the different output scenarios in the attached tests.

I also patched a minor bug with the test output assertions script.

Copy link

google-cla bot commented Sep 25, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@luispadron
Copy link
Contributor

Thanks for contributing!

FWIW there's some commits in upstream adding support for this we'll likely take. Someone just needs to do the cherry-picks

@ed-irl
Copy link
Author

ed-irl commented Sep 25, 2024

Thanks for the heads up. @luispadron do you happen to know if there will be a similar update to rules_apple? I was going to submit an almost identical patch there momentarily...

@luispadron
Copy link
Contributor

This is the upstream commit i mean: 26e2624

You can find several other Swift Testing related commits we need to cherry-pick over in: https://github.com/bazelbuild/rules_swift/commits/upstream

I havent checked rules_apple yet but that also has an upstream branch you can take a look at. We'd prefer to cherry pick changes so future cherry picks are easier, if you want to help there that would be awesome!

@ed-irl
Copy link
Author

ed-irl commented Sep 25, 2024

@luispadron thanks for the info. I'm not familiar with this style of git development (having an upstream branch that features are cherry-picked off of vs just merging as one would typically). I'd be interested to help. Just to clarify, are you suggesting to just literally cherry-pick the commit your referenced (as well as any others that are needed) and then make my own PR with those commits instead?

@luispadron
Copy link
Contributor

Just to clarify, are you suggesting to just literally cherry-pick the commit your referenced (as well as any others that are needed) and then make my own PR with those commits instead?

Yeah exactly, there will likely be merge conflicts to deal with but this helps us maintain mostly similar code to Googles upstream.

See #1311 as an example.

@ed-irl
Copy link
Author

ed-irl commented Sep 25, 2024

@luispadron it looks like the architecture of the swift_test rule on upstream has diverged quite a bit (master no longer has the test wrapper script and seems to be doing something else inside the rule). I suspect bringing the master branch in sync with upstream will be a nontrivial piece of work, and that someone who has greater understanding of the codebase's history would be better positioned to cherry pick this change and the dependencies that are needed off of upstream.

In light of that, how would you like to proceed? I'd be happy to talk on a GVC if that would be helpful too.

@brentleyjones
Copy link
Collaborator

I'm currently churning through some cherry-picks. Hopefully I can reach that change, or close to it, before I need to move onto something else.

@brentleyjones
Copy link
Collaborator

Locally I have the cherry-pick applied, just churning through PRs to get to it. Hopefully it won't be much longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants