Created `ReceiptParser` SPM by NachoSoto · Pull Request #2155 · RevenueCat/purchases-ios

NachoSoto · 2022-12-14T21:16:14Z

Finishes CSDK-17.
Follow up to #2153. Extracted from #2096.

Changes:

Created target in Package.swift. It can be built separately: swift build --target ReceiptParser
Created ReceiptParser target in project:
Added ReceiptParser scheme to allow building the target from Xcode:
Added CI spm-receipt-parser job
Added CI installation-tests-receipt-parser job
Added ReceiptParserAPITester to test API for ReceiptParser's target
Compile ReceiptParserAPITester when running RevenueCat tests so these are validated as well. -

NachoSoto · 2022-12-14T21:55:25Z

We need some files specific to just the ReceiptParser module (right now the separate LoggerType).

Should this be ** to match within subdirectories? Just in case we ever addd directories to that folder

This works 👍🏻 changed.

tonidero

I'm not very familiar with creating SPM packages, but code LGTM. @aboedo in case you want to take a look.

tonidero · 2022-12-22T07:57:20Z

+import Foundation
+import ReceiptParser
+
+func checkAppleReceiptAPI(_ receipt: AppleReceipt! = nil) {


These API tests seem kinda duplicated with the ones in the other project . I wonder if we could avoid the duplication creating a symlink or something like that? Not sure if that would work.

Or maybe remove them from the other project?

Here's my reasoning:

Keeping both: we have these APIs exposed as part of RevenueCat and ReceiptParser, so we need to make sure that they're stable in each

Keeping them separate: this file is checking the API for ReceiptParser.PurchasesReceiptParser. The other file checks RevenueCat.PurchasesReceiptParser API. Obviously we share code internally, but the interfaces might not always coincide. Purchases.ReceiptParser might have more methods in the future because it's part of the bigger SDK.

Keeping them separate: this file is checking the API for ReceiptParser. The other file checks RevenueCat's PurchasesReceiptParser API. Obviously we share code internally, but the interfaces might not always coincide. Purchases.ReceiptParser might have more methods in the future because it's part of the bigger SDK.

Hmm well, do you have in mind any possible future cases of this scenario? If we do I think it's ok but if not, I would prefer avoiding duplication now for an uncertain scenario.

I'm not sure how to do this in a clean way though. The files might be similar but not quite. One has to import RevenueCat and the other import ReceiptParser.

Hmm right... I didn't think about that. In that case, I think it's ok to leave as is. Don't want to have a complex setup for the sake of avoiding a bit of duplication.

It was a good suggestion though. But yeah not sure it's worth it since we'd need to have some way of dynamically copying contents of the file, etc. etc.

tonidero · 2022-12-22T08:00:09Z

+
+    var body: some Scene {
+        WindowGroup {
+            EmptyView()


Hmm, do we need this file? I guess we need the project in order to have the tests, but not sure if the project requires an app class like this which doesn't seem useful right now.

I made this project as slim as possible. The other integration test apps have even more stuff in them.

tonidero · 2022-12-22T08:01:48Z

+import ReceiptParserIntegration
+import XCTest
+
+final class ReceiptParserIntegrationTests: XCTestCase {


Should we add a success test case here?

I'd need to have a sample receipt in here, which are pretty large.
The actual logic for parsing receipts is already tested in unit tests. This is an installation test, so I think simply checking that the method can be called and it does what it's supposed to is enough.

tonidero · 2022-12-22T08:02:30Z

+
+final class ReceiptParserIntegrationTests: XCTestCase {
+
+    func testCanParseReceipts() throws {


Maybe rename to testParsingEmptyReceiptThrowsError or something like that? In case we add more tests in the future.

Yeah good point.

vegaro

Looks good to me! I guess you've tried installing it locally or from this branch and it works fine 😄

NachoSoto · 2022-12-22T17:57:49Z

I guess you've tried installing it locally or from this branch and it works fine 😄

Yeah, and we even have an installation test :)
This is what it looks like when you install it through SPM:

Finishes [CSDK-17]. - Created target in `Package.swift` - Created `ReceiptParser` target in project - Added CI `spm-receipt-parser` job - Added CI `installation-tests-receipt-parser` job - Added `ReceiptParser` scheme to allow building the target from Xcode - Added `ReceiptParserAPITester` to test API for `ReceiptParser`'s target - Compile `ReceiptParserAPITester` when running `RevenueCat` tests so these are validated as well.

@NachoSoto

**This is an automatic release.** ### New Features * Created `ReceiptParser` SPM (#2155) via NachoSoto (@NachoSoto) * Exposed `PurchasesReceiptParser` and `AppleReceipt` (#2153) via NachoSoto (@NachoSoto) ### Bugfixes * `Restore purchases`: post product data when posting receipts (#2178) via NachoSoto (@NachoSoto) ### Other Changes * Added `Dictionary.merge` (#2190) via NachoSoto (@NachoSoto) * `CircleCI`: use Xcode 14.2.0 (#2187) via NachoSoto (@NachoSoto) * `ReceiptParser`: a few documentation improvements (#2189) via NachoSoto (@NachoSoto) * `Purchase Tester`: fixed `TestFlight` deployment (#2188) via NachoSoto (@NachoSoto) * `Purchase Tester`: display specific `IntroEligibilityStatus` (#2184) via NachoSoto (@NachoSoto) * `Purchase Tester`: fixed `SubscriptionPeriod` (#2185) via NachoSoto (@NachoSoto)

NachoSoto added the pr:feat A new feature label Dec 14, 2022

NachoSoto requested a review from a team December 14, 2022 21:16

NachoSoto commented Dec 14, 2022

View reviewed changes

Comment thread .../ReceiptParserInstallation/ReceiptParserIntegrationTests/ReceiptParserIntegrationTests.swift Outdated

NachoSoto mentioned this pull request Dec 14, 2022

[WIP] Separate ReceiptParser SPM Package #2096

Closed

11 tasks

NachoSoto force-pushed the receipt-parser-public branch 2 times, most recently from 43171dd to a9ae4cd Compare December 14, 2022 21:40

NachoSoto force-pushed the receipt-parser-spm-4 branch from 357968c to 194f350 Compare December 14, 2022 21:43

NachoSoto force-pushed the receipt-parser-public branch from a9ae4cd to d356639 Compare December 14, 2022 21:46

NachoSoto force-pushed the receipt-parser-spm-4 branch 3 times, most recently from 726312b to 5e0c5ca Compare December 14, 2022 21:53

NachoSoto commented Dec 14, 2022

View reviewed changes

NachoSoto force-pushed the receipt-parser-public branch from d356639 to 0ff0269 Compare December 15, 2022 18:26

NachoSoto force-pushed the receipt-parser-spm-4 branch 9 times, most recently from fbfec51 to bdc4073 Compare December 15, 2022 20:24

NachoSoto force-pushed the receipt-parser-public branch from 0ff0269 to 1b9e77a Compare December 20, 2022 18:08

NachoSoto force-pushed the receipt-parser-spm-4 branch from bdc4073 to 93e6b19 Compare December 20, 2022 18:09

tonidero mentioned this pull request Dec 21, 2022

Exposed PurchasesReceiptParser and AppleReceipt #2153

Merged

tonidero approved these changes Dec 22, 2022

View reviewed changes

vegaro approved these changes Dec 22, 2022

View reviewed changes

NachoSoto force-pushed the receipt-parser-public branch from 1b9e77a to 05d4883 Compare December 22, 2022 17:59

Base automatically changed from receipt-parser-public to main December 22, 2022 18:09

NachoSoto force-pushed the receipt-parser-spm-4 branch from 93e6b19 to 9220755 Compare December 22, 2022 18:27

NachoSoto enabled auto-merge (squash) December 22, 2022 18:36

NachoSoto added 5 commits December 22, 2022 11:29

Added installation-tests-receipt-parser job

32db724

Fixed project signing

d5a3db2

Updated test name

e2a235b

Using **

1aae13d

NachoSoto force-pushed the receipt-parser-spm-4 branch from 9220755 to 1aae13d Compare December 22, 2022 19:29

NachoSoto merged commit 6d78f2b into main Dec 22, 2022

NachoSoto deleted the receipt-parser-spm-4 branch December 22, 2022 20:02

RCGitBot mentioned this pull request Dec 27, 2022

[AUTOMATIC] Release/4.16.0 #2191

Merged


		final class ReceiptParserIntegrationTests: XCTestCase {

		func testCanParseReceipts() throws {

Conversation

NachoSoto commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vegaro left a comment

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Dec 14, 2022 •

edited

Loading

NachoSoto Dec 22, 2022 •

edited

Loading

NachoSoto Dec 22, 2022 •

edited

Loading

NachoSoto commented Dec 22, 2022 •

edited

Loading