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

Re-added the RevenueCatUI tests job on every commit #4113

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Jul 24, 2024

I realized that we weren't running the suite of tests from RevenueCatUI in every commit.

I missed this one because the name made it seem like it was a test specific to SPM, but it's just that it could only be run through SPM at the time.

It can now be run from Xcode, but that's a separate story, so for now this just re-adds it and clarifies the naming a bit

@aboedo aboedo added the ci label Jul 24, 2024
@aboedo aboedo self-assigned this Jul 24, 2024
@aboedo
Copy link
Member Author

aboedo commented Jul 24, 2024

Oh, I guess re-doing the names sadly means that I'd have to also update the name in the required jobs list.
I'll leave it as is for now until folks share their thoughts on the naming

@aboedo aboedo requested a review from a team July 24, 2024 21:53
Comment on lines 1282 to 1289
- lint:
name: "swiftlint"
- run-test-ios-17:
name: "Unit tests"
- pod-lib-lint:
name: "Pod lib lint"
- spm-revenuecat-ui-ios-17:
name: "RevenueCatUI tests"
Copy link
Member Author

Choose a reason for hiding this comment

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

let me know if the name feels like it's not adding much and / or making it harder to map the jobs as they show up in github to the circleCI config. I just did this because the name spm-revenuecat-ui-ios-17 seemed so specific that it didn't seem like a vital check to have until I realized what it was

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm how about renaming the job to something like run-revenuecat-ui-ios-17 instead? I feel changing the name like this might make it harder to try to find what job is failing later.

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

@aboedo
Copy link
Member Author

aboedo commented Jul 25, 2024

@RCGitBot please test

@aboedo aboedo requested a review from tonidero July 26, 2024 18:47
@aboedo aboedo enabled auto-merge (squash) July 26, 2024 18:47
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Just a comment but looks good!

@@ -1269,7 +1269,7 @@ workflows:
jobs:
- spm-revenuecat-ui-ios-15
- spm-revenuecat-ui-ios-16
Copy link
Contributor

Choose a reason for hiding this comment

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

I do wonder if we should also rename these, if they are doing the same thing but on different iOS versions...

@aboedo aboedo merged commit 73208b2 into main Jul 29, 2024
5 checks passed
@aboedo aboedo deleted the andy/re-add-some-test-jobs branch July 29, 2024 08:21
jamesrb1 pushed a commit that referenced this pull request Aug 7, 2024
I realized that we weren't running the suite of tests from RevenueCatUI
in every commit.

I missed this one because the name made it seem like it was a test
specific to SPM, but it's just that it could only be run through SPM at
the time.

It can now be run from Xcode, but that's a separate story, so for now
this just re-adds it and clarifies the naming a bit
MarkVillacampa pushed a commit that referenced this pull request Aug 7, 2024
**This is an automatic release.**

### Bugfixes
* Fix Paywalls crash on iOS 18 beta (#4154) via Andy Boedo (@aboedo)
### Dependency Updates
* Bump danger from 9.4.3 to 9.5.0 (#4143) via dependabot[bot]
(@dependabot[bot])
* Bump nokogiri from 1.16.6 to 1.16.7 (#4129) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane from 2.221.1 to 2.222.0 (#4130) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Update deployment targets for tests (#4145) via Andy Boedo (@aboedo)
* Deploy purchaserTester: clean up dry-run parameter (#4140) via Andy
Boedo (@aboedo)
* Clean up API Testers (#4141) via Andy Boedo (@aboedo)
* More project structure cleanup (#4131) via Andy Boedo (@aboedo)
* temporarily disables purchasetester deploy (#4133) via Andy Boedo
(@aboedo)
* Fix trigger all tests branch (#4135) via Andy Boedo (@aboedo)
* Clean up XCWorkspace and testing apps (#4111) via Andy Boedo (@aboedo)
* tests trigger: add target-branch parameter to trigger from the right
branch (#4121) via Andy Boedo (@aboedo)
* Re-added the RevenueCatUI tests job on every commit (#4113) via Andy
Boedo (@aboedo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants