Skip to content

Add support for tests executed repeatedly#585

Merged
stephencelis merged 1 commit into
pointfreeco:mainfrom
krzysztofpawski:support_tests_iterations
Sep 22, 2022
Merged

Add support for tests executed repeatedly#585
stephencelis merged 1 commit into
pointfreeco:mainfrom
krzysztofpawski:support_tests_iterations

Conversation

@krzysztofpawski

@krzysztofpawski krzysztofpawski commented Mar 23, 2022

Copy link
Copy Markdown
Contributor

Motivation

Xcode 13 introduced new parameters to xcodebuild that allow to run tests repeatedly (wwdc talk).
Unfortunately SnapshotTesting doesn't handle such situation in a correct way.

Proposal

We can use XCTestObservation in order to clean counter map between different test cases.

Resolved issues

Closes #577

@krzysztofpawski krzysztofpawski force-pushed the support_tests_iterations branch 5 times, most recently from 422288f to 44b3736 Compare March 24, 2022 12:20
Comment thread Sources/SnapshotTesting/AssertSnapshot.swift Outdated
@tahirmt

tahirmt commented Mar 25, 2022

Copy link
Copy Markdown
Contributor

I have two questions regarding this

  1. Would we still have SnapshotTesting increment the counter for multiple snapshots inside one test case? e.g. snapshot assertion from different lines but the same test case
  2. I'm using Quick/Nimble for this and if the test originates from the same line, i.e. a toEventually expectation, would it use the same file name for each execution?

Comment thread Sources/SnapshotTesting/AssertSnapshot.swift
@krzysztofpawski krzysztofpawski force-pushed the support_tests_iterations branch 3 times, most recently from aad695f to cae4380 Compare March 28, 2022 06:20
@krzysztofpawski

Copy link
Copy Markdown
Contributor Author

I have two questions regarding this

  1. Would we still have SnapshotTesting increment the counter for multiple snapshots inside one test case? e.g. snapshot assertion from different lines but the same test case

Yes. TestObserver is called after test case is executed so the feature of counting multiple assertSnapshot calls works as expected.

  1. I'm using Quick/Nimble for this and if the test originates from the same line, i.e. a toEventually expectation, would it use the same file name for each execution?

I've never used Quick or Nimble. Would you be so kind to check with my forked version if it works with your project? I've already checked with our where we have planty of different scenarios but definitely would be cool to have confirmation also from someone else.

@tahirmt

tahirmt commented Mar 28, 2022

Copy link
Copy Markdown
Contributor

@krzysztofpawski I tried it out and it still produces many snapshots. The way Quick works is that it executes the same condition with a polling until it passes.

@tahirmt

tahirmt commented Mar 29, 2022

Copy link
Copy Markdown
Contributor

Maybe it will make sense to make it unique based on the test line. The only thing that would break it is if someone adds two assertions on the same line

@krzysztofpawski krzysztofpawski force-pushed the support_tests_iterations branch from cae4380 to 922eee3 Compare March 30, 2022 06:36
@krzysztofpawski

Copy link
Copy Markdown
Contributor Author

@tahirmt could you check it one more time? I found one more case when it was producing too many reference files in our repo.
If not maybe then you could prepare a very simple project reproducing the issue mentioned by you and attach it here?

@tahirmt

tahirmt commented Mar 30, 2022

Copy link
Copy Markdown
Contributor

@krzysztofpawski I created a demo repo to test this using your fork https://github.com/tahirmt/test-toeventually-behavior-snapshots.

@krzysztofpawski

Copy link
Copy Markdown
Contributor Author

@tahirmt I'm afraid it can't be fixed in swift-snapshot-testing library without some bigger refactor. But the solution I see for you is that you can use named argument (pass some non empty string) to omit this counting mechanism. It's not perfect but it should do the work for your matcher.

@tahirmt

tahirmt commented Mar 30, 2022

Copy link
Copy Markdown
Contributor

@tahirmt I'm afraid it can't be fixed in swift-snapshot-testing library without some bigger refactor. But the solution I see for you is that you can use named argument (pass some non empty string) to omit this counting mechanism. It's not perfect but it should do the work for your matcher.

That's what I was thinking too. I was thinking of many alternatives but I can't think of a better solution for now. Thanks for looking into it though.

@krzysztofpawski

Copy link
Copy Markdown
Contributor Author

@stephencelis any possibility to review this PR?

@stephencelis stephencelis merged commit f29e201 into pointfreeco:main Sep 22, 2022
niil-qb pushed a commit to quickbit/swift-snapshot-testing that referenced this pull request Feb 23, 2023
NachoSoto added a commit to RevenueCat/purchases-ios that referenced this pull request Apr 26, 2023
This includes [this fix](pointfreeco/swift-snapshot-testing#585) which should solve some of the flaky test failures we've been seeing lately, which made test retries also fail.
We couldn't update to 1.10.0 because it dropped support for iOS 12.0, but that was fixed in pointfreeco/swift-snapshot-testing#698
NachoSoto added a commit to RevenueCat/purchases-ios that referenced this pull request Apr 26, 2023
This includes [this
fix](pointfreeco/swift-snapshot-testing#585)
which should solve some of the flaky test failures we've been seeing
lately, which made test retries also fail.

We couldn't update to 1.10.0 because it dropped support for iOS 12.0,
but that was fixed in
pointfreeco/swift-snapshot-testing#698

This also reverts #2426, since that's no longer required now that
retries should work properly.
OksanaFedorchuk pushed a commit to urlaunched-com/swift-snapshot-testing that referenced this pull request Mar 28, 2024
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.

Running test repeatedly results in duplicate snapshot being created.

4 participants