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 FXIOS-10094 [Unit Tests] Add store utility for tests #22172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cyndichin
Copy link
Contributor

📜 Tickets

Jira ticket
Github issue

💡 Description

Create StoreTestUtility that can be conformed by test classes that want to override the global store with a testable store.
Add StoreTestUtilityHelper which is a concrete class that can be used by the test class.

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Sep 24, 2024

Messages
📖 Edited 2 files
📖 Created 1 files

Generated by 🚫 Danger Swift against fc2e13a

@cyndichin cyndichin marked this pull request as ready for review September 24, 2024 18:25
@cyndichin cyndichin requested a review from a team as a code owner September 24, 2024 18:25
import Redux
@testable import Client

protocol StoreTestUtility {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my opinion i think we might not need this protocol, i'd just add the StoreTestUtilityHelper directly as property of the Tester class. Let me know what you think! thanks @cyndichin 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment! Hmm, I wanted a way to ensure that any test that uses the store will conform to the protocol, which will throw compiler errors if the user does not have the setup or reset methods and then can use the StoreTestUtilityHelper for convenience. Happy to remove if you don't think it's useful though.

Does it seem unnecessary extra code for you? Do you think we should open up to the team?

func resetTestingStore()
}

// Utility class used when replacing the global store for testing purposes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: i'd add a third / so it is also visible for documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated! will do this going forward and use ///

any cases where you'll use // over ///? Do you think this would be a good thing to standardize?


// In order to avoid flaky tests, we should reset the store
// similar to production
func resetTestingStore() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! updated!

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.

3 participants