Skip to content

SKTestSession: finish all unfinished transactions before starting each test#2066

Merged
NachoSoto merged 2 commits into
mainfrom
test-session-clear-transactions
Nov 26, 2022
Merged

SKTestSession: finish all unfinished transactions before starting each test#2066
NachoSoto merged 2 commits into
mainfrom
test-session-clear-transactions

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Filed this as FB11799675. I noticed some tests began with prior transactions (despite using SKTestSession.clearTransactions), so this workaround ensures each test begins in a clean state.

@NachoSoto NachoSoto added the test label Nov 17, 2022
@NachoSoto NachoSoto requested a review from a team November 17, 2022 18:59

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved this from StoreKitTestHelpers.swift into this file since this is only for StoreKitConfigTestCase and not relevant for integration tests.

@NachoSoto NachoSoto force-pushed the test-session-clear-transactions branch from 6039819 to 7ef2d91 Compare November 17, 2022 19:53
@NachoSoto

NachoSoto commented Nov 17, 2022

Copy link
Copy Markdown
Contributor Author

I think this is exposing CSDK-517 even more frequently.
(See #2055).

@NachoSoto NachoSoto changed the title SKTestSession: finish all unfinished transactions when beginning tests SKTestSession: finish all unfinished transactions before beginning tests Nov 18, 2022
@NachoSoto NachoSoto changed the title SKTestSession: finish all unfinished transactions before beginning tests SKTestSession: finish all unfinished transactions before starting each test Nov 18, 2022
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Actually, I wonder if this will make that issue better (see https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/8929/workflows/e990dfcf-effb-4923-b688-1e3655aee62a/jobs/44610).
I bet there are pending transactions, and that's why even retrying triggers the same issue.

Comment thread Tests/BackendIntegrationTests/BaseBackendIntegrationTests.swift Outdated
Filed this as `FB11799675`. I noticed some tests began with prior transactions (despite using `SKTestSession.clearTransactions`), so this workaround ensures each test begins in a clean state.
@NachoSoto NachoSoto force-pushed the test-session-clear-transactions branch from d5a10c5 to af643ba Compare November 26, 2022 00:09
@NachoSoto NachoSoto enabled auto-merge (squash) November 26, 2022 00:09
@NachoSoto NachoSoto merged commit cd4e33c into main Nov 26, 2022
@NachoSoto NachoSoto deleted the test-session-clear-transactions branch November 26, 2022 00:20
NachoSoto added a commit that referenced this pull request Nov 28, 2022
I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

So all these new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK.

Depends on #2064 and #2066.
NachoSoto added a commit that referenced this pull request Dec 15, 2022
I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

So all these new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK.

Depends on #2064 and #2066.
NachoSoto added a commit that referenced this pull request Dec 15, 2022
)

See [TRIAGE-179].

I wrote all these to cover the change in #2063. I wrote a failing test
that only passed with the change in #2063. However, when running that in
SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ
from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the
moment you access the sequence. The sequence emits a finite number of
transactions. If the App Store processes new transactions for the user
while you’re accessing this sequence, the new transactions appear in the
transaction listener, updates.

These new tests verify the behavior of the SDK when encountering
transactions created prior to initializing the SDK, and some made after
initialization.

Integration tests now are as follows:
- `BaseStoreKitIntegrationTests`
  - `StoreKit1IntegrationTests`
  - `StoreKit2IntegrationTests`
  - `BaseStoreKitObserverModeIntegrationTests`:
    - `StoreKit1ObserverModeIntegrationTests`
    - `StoreKit2ObserverModeIntegrationTests`
    - `StoreKit1ObserverModeWithExistingPurchasesTests`
    - `StoreKit2ObserverModeWithExistingPurchasesTests`

Depends on #2064, #2066, and #2134.

[TRIAGE-179]:
https://revenuecats.atlassian.net/browse/TRIAGE-179?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
NachoSoto added a commit that referenced this pull request Dec 20, 2022
See #2066.

I saw a bunch of these messages:
> DEBUG: ℹ️ Finishing 0 transactions before running tests

Which doesn't add anything if there aren't any leftover transactions.
NachoSoto added a commit that referenced this pull request Dec 20, 2022
See #2066.

I saw a bunch of these messages:
> DEBUG: ℹ️ Finishing 0 transactions before running tests

Which doesn't add anything if there aren't any leftover transactions.
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.

2 participants