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

feat: make wizards testable #29

Merged
merged 9 commits into from
Jun 3, 2022
Merged

Conversation

ju5t
Copy link
Contributor

@ju5t ju5t commented Jun 1, 2022

This makes two macro's available for developers:

  • emitEvents: can be used to pass events from StepComponents to the wizard.
  • getStepState: can be used on a wizard to fetch state for a step to pass on.

I wasn't completely sure how and where to add individual tests for these macro's. Both are used in tests at this moment. It could use an exception in getStateForStep when a step does not exist, I can add that in a second commit (or you can merge it and add it yourself).

getStepState found an error in how we test the initial state. While the initial state is available in the wizard, if the public property doesn't exist in the step itself it's not going to be used. I've added an extra test for it.

@freekmurze
Copy link
Member

Thanks for you work on this. Seems like the test are failing, could you take a look at that?

Could you also add a page on testing to the docs where you explain how these macro's could be used?

@ju5t
Copy link
Contributor Author

ju5t commented Jun 1, 2022

@freekmurze I don't know why the test is failing on prefer-stable. I cannot replicate it on my machine. I haven't used the snapshot package before so I'm not sure where to look for.

About the docs: absolutely. I will do this tomorrow; and I will add the exception too.

@ju5t ju5t marked this pull request as draft June 1, 2022 18:07
@freekmurze
Copy link
Member

I don't know why the test is failing on prefer-stable. I cannot replicate it on my machine. I haven't used the snapshot package before so I'm not sure where to look for.

Don't worry about the tests, I know what the issue is. Just make sure they are running well locally and I'll take care of it when you're done with this PR 👍

About the docs: absolutely. I will do this tomorrow; and I will add the exception too.

👍

@ju5t ju5t changed the title feat: public emitEvents and getStateForStep macro feat: make wizards testable Jun 1, 2022
@ju5t ju5t marked this pull request as ready for review June 1, 2022 20:56
@ju5t
Copy link
Contributor Author

ju5t commented Jun 1, 2022

@freekmurze decided to fix it this evening, should be good to go now. Let me know if something can be improved.

@ju5t
Copy link
Contributor Author

ju5t commented Jun 2, 2022

I'm tempted to rename the getStateForStep method to getState and make the step optional. This would allow us to test all state as well.

Edit: I went for getStepState as it's similar to how we use setStepState in the wizard itself. I'm a bit in doubt if it should support fetching all state too or if this warrants an additional macro. What do you think @freekmurze?

@freekmurze freekmurze merged commit 64aa877 into spatie:main Jun 3, 2022
@freekmurze
Copy link
Member

Thanks!

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