-
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
Unit tests #19
Unit tests #19
Conversation
Thanks a lot! I will look into it in the upcoming weeks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the base branch to 13_unit_tests
and added some comments. Thanks!
src/components/Button.spec.js
Outdated
}); | ||
|
||
test('show alert when the button gets clicked', async () => { | ||
const { getByTestId, findByRole } = render(Button, {title: 'testButton'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the spirit of the guiding principles, it is recommended to use this only after the other queries don't work for your use case. Using data-testid attributes do not resemble how your software is used and should be avoided if possible. (https://testing-library.com/docs/queries/bytestid)
I would recommend to use getByRole
as the button
element has the default role button
src/components/Button.svelte
Outdated
@@ -4,7 +4,7 @@ | |||
export let title = ''; | |||
</script> | |||
|
|||
<button title="{title}" disabled="{disabled}" on:click="{buttonAction}"> | |||
<button data-testid="testButton" title="{title}" disabled="{disabled}" on:click="{buttonAction}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we use getByRole
we also don't need to introduce data-testid
.babelrc
Outdated
@@ -0,0 +1,6 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need both the file .babelrc
and babel.config.js
? Can we delete .babelrc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I think so. I will delete it.
babel.config.js
Outdated
@@ -0,0 +1,6 @@ | |||
module.exports = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to rename babel.config.js
to babel.config.cjs
to make it work. Otherwise I get error error while loading config - You appear to be using a native ECMAScript module configuration file, which is only supported when running Babel asynchronously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works for me either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed my PR – the next test goes to you ;-)
Hope you are fine? |
Thanks a lot! |
Hi Malte,
you can find an example unit test in this pull request.
regards
Kristian