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

Consolidate test store with global app store #2756

Conversation

mstrofbass
Copy link
Collaborator

@mstrofbass mstrofbass commented Jan 6, 2025

My theory is that the reason the enhancers wouldn't work with the test store is because of some weird import issues that resulted in the real createStore being called in src/stores/app.ts. We can bypass fixing #2336 and #2337 by just going straight to the global store.

This change does the following:

  1. Adds a helper for clearing the store
  2. Adds a helper for skipping the tutorial
  3. Adds a helper for initializing the store that defaults to clearing the store and skipping the tutorial
  4. Replaces all store references that originate with createTestStore with references directly to the store
  5. Calls initStore in the beforeEach when appropriate.

Closes #2336
Closes #2337
Closes #2338

Copy link
Contributor

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me. It accomplishes the goal of consolidating the test store with the global app store.

At this time there appears to be little difference between a single beforeEach(initStore) and a paired beforeEach(skipTutorial) and afterEach(clear), so I'm fine with the single initStore. We can keep them combined until there is a need otherwise.

src/test-helpers/clearStore.ts Outdated Show resolved Hide resolved
src/test-helpers/skipTutorial.ts Outdated Show resolved Hide resolved
src/test-helpers/initStore.ts Show resolved Hide resolved
@raineorshine
Copy link
Contributor

Hi @mstrofbass. Just checking in on this. Could you take a look at the few small changes requested above? Then we should be able to get this merged. Thanks!

@mstrofbass mstrofbass force-pushed the consolidate-test-store-with-global-app-store3-2338 branch from f2367ca to 2bf4d6f Compare January 29, 2025 05:32
@mstrofbass mstrofbass marked this pull request as ready for review January 29, 2025 05:55
@mstrofbass
Copy link
Collaborator Author

Okay, should be good to go now.

@raineorshine raineorshine merged commit 395c394 into cybersemics:main Jan 29, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants