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 tests executed repeatedly #585

Merged

Conversation

krzysztofpawski
Copy link
Contributor

@krzysztofpawski krzysztofpawski commented Mar 23, 2022

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
@tahirmt
Copy link
Contributor

tahirmt commented Mar 25, 2022

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?

@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
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
Copy link
Contributor

tahirmt commented Mar 28, 2022

@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
Copy link
Contributor

tahirmt commented Mar 29, 2022

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
Copy link
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
Copy link
Contributor

tahirmt commented Mar 30, 2022

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

@krzysztofpawski
Copy link
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
Copy link
Contributor

tahirmt commented Mar 30, 2022

@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
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
Muhieddine-El-Kaissi pushed a commit to thumbtack/swift-snapshot-testing that referenced this pull request Aug 8, 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