|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago | ||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This also isn't related to the test. It's best to narrow the scope of a unit test to the smallest possible piece of functionality - we want to know exactly what's not working when it fails, but adding all these pre-check assertions makes it more difficult to figure out what's going wrong. I recommend dropping this whole block, as well. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This isn't a test for CSRF token functionality, so you can drop this whole block | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This block can also be dropped, it's not a test about usernames being unique. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This isn't a test about the email needing to be unique (as an aside, HTTP 200 is almost certainly not the right error code) so this block can go, too. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This block can be dropped. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This block can be dropped. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This block can be dropped. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This would probably be a good test all by itself to make sure, for example, it doesn't just accept any non-ascii password, but it's not something we should combine with a test about whether it's possible to sign up and authenticate with a non-ascii password. | ||
|
||
farhaan commented 6 years ago it should accept any non-ascii character as password right ? | ||
jcline commented 6 years ago Yes. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This block can also be dropped. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
jcline commented 6 years ago This comment doesn't seem relevant? I'm not seeing an error being asserted below (it looks like the block below tests that it's possible to log in, which is what we want) | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
The above changes encode the password to UTF_8
to make the password
compatible for ascii as well as non-ascii character.
Signed-off-by: Farhaan Bukhsh farhaan.bukhsh@gmail.com
This test isn't really related to the web form accepting a non-ASCII password. It'd be good to be its own test, perhaps, although I expect other tests are also running this.