-
Notifications
You must be signed in to change notification settings - Fork 25
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 Tests For Solid Toast #15
Comments
Do you refer to unit tests (for certain functions....), or end-2-end tests for the full functionality ? is jest / cypress what you're looking for respectively. |
Apologies for the late reply. Been talking a break from some of the projects this week. I believe my concern recently has been while making changes/updates to the core API, I want to make sure that backwards compatibility is correctly preserved. The current pain points that I can recognise right now is that // Toaster config setting all toasts to have a default duration of 3secs
<Toaster
toastOptions={{
duration: 3000
}}
/>
// Call-Site toast invocation that overrides the 3 seconds default
toast('Hello, World', {
duration: 6000
}) Along with this. Different toast types have different default durations if nothing is configured https://github.com/ardeora/solid-toast/blob/main/src/core/defaults.ts#L4-L10. These defaults should only be applied if the Toaster Component/toast function do not provide any So as per priority goes. Having Jest unit tests that test different edge cases and configuration permutations would be a first good step for making the library resilient to most common source of bugs. End-To-End tests would be awesome to have but that is something that can be put in the |
Thank you, yes it does, I haven't checked the full codebase yet since I wasn't sure I'll be working on this, tomorrow i'll start and whenever i'm done, i'll send a pull-request. |
There are multiple combinations of ways to use/configure solid-toast. We need to add tests to make sure the configuration follows the correct hierarchy and manages edge cases correctly.
The text was updated successfully, but these errors were encountered: