-
Notifications
You must be signed in to change notification settings - Fork 97
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
Signup flow #246
Signup flow #246
Conversation
shreya-mishra
commented
Aug 5, 2022
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.
Congrats on learning testing. I have added comments please a have look.
@@ -47,6 +47,7 @@ | |||
{{/each}} | |||
{{#if this.isSubmitDisabled }} |
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 think we can achieve both the buttons with the if
condition on the disabled attribute
tests/acceptance/signup-flow-test.js
Outdated
assert.dom('[data-test-id="form-input-first_name"]').hasProperty('value','test') | ||
assert.dom('[data-test-id="form-input-last_name"]').hasProperty('value','user') |
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.
hasProperty
checks the properties of the selected HTML element, hasProperty
is used to check let's say whether the input field is a type password or a normal input field etc.
To check the value of input hasValue
is the assertion to use.
tests/acceptance/signup-flow-test.js
Outdated
|
||
setupApplicationTest(hooks); | ||
|
||
test('http://localhost:4200/signup', async function(assert) { |
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 think a more descriptive name test name will be good instead of http://localhost:4200/signup
tests/acceptance/signup-flow-test.js
Outdated
setupApplicationTest(hooks); | ||
|
||
test('http://localhost:4200/signup', async function(assert) { | ||
const fields = { |
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.
nit: We can move fields
to mocks folder.
tests/acceptance/signup-flow-test.js
Outdated
assert.dom('[data-test-id="signup-button"]').exists() | ||
|
||
await this.pauseTest() | ||
window.alert = originalWindowAlert; |
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 does the test have an alert? 🤔
What does this do for our assertions?
@@ -1,6 +1,7 @@ | |||
<div class="field"> | |||
<label for={{@for}}>{{@label}}:</label> | |||
<Input | |||
data-test-id="form-input-{{@id}}" |
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 can be a static named id, so that we can easily grab it in the tests.
data-test-id="form-input-{{@id}}" | |
data-test-id="form-input" |
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.
On starting with testing 🤩
"Welcome to the mafia"
tests/acceptance/signup-flow-test.js
Outdated
} | ||
|
||
const originalWindowAlert = window.alert; | ||
window.alert = function() { return true;} |
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.
do we need this?
(im not sure 100% honestly)
|
||
assert.dom('[data-test-id="signup-button-disabled"]').doesNotExist() | ||
assert.dom('[data-test-id="signup-button"]').exists() | ||
|
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.
can there also be something like clicking the button and then disabling it, and checking for the same?
New PR has been raised for the same issue #269 |