forgot password is broken

[bukosabino]

If you enable javascript in your browser the link is broken.

You can work if you write the url ​http://127.0.0.1:6543/auth/forgot_password/

[bukosabino]

it's resolved.

event "click" was associated to tag <a id="forgot_password">, which did not work good.

I removed, because it really is not necessary.

[bukosabino]

sorry, the address:
/mediagoblin/static/js/autofilledin_password.js

We can to notify to live instances. They have this problem.
​http://wiki.mediagoblin.org/Live_instances

[Elrond]

It's not fixed, at least not for me. So re-opening.

Go to /, then cklick "Log in", then click "Forget Password".

The following works for me. But I'm not sure on js things. And we don't have a unit test for this.

--- a/mediagoblin/static/js/autofilledin_password.js
+++ b/mediagoblin/static/js/autofilledin_password.js
@@ -19,7 +19,7 @@
 $(document).ready(function(){
   $('#forgot_password').click(function(event){
     event.preventDefault();
-    window.location.pathname = $(this).attr('href') + '?username=' +
+    window.location = $(this).attr('href') + '?username=' +
                                $('#username').val();
   });
 });

[Jakob Kramer]

Your patch makes sense! Please commit this. :) pathname is wrong when you put the whole URL into it. Firefox just did what I expected, so I thought it was correct, but other browsers behave correctly.

[Elrond]

Okay. So.

1. Good that my patch is the right thing. So we have way to fix the problem at hand.
2. I would like to not apply this patch NOW, so that we can have a failing unit/functional test written first (that's the target of this bug now).
3. If (badly enough likely) we don't get the unit test before the next release, we'll apply this patch a week before the next release and keep the bug open to create the unit test.

Does that make sense?

[Gabriela Thumé]

In ticket #458 we are talking about using Selenium to functional tests. Because of that I tried to create one test based on this broken link of forgot password at ​https://gist.github.com/GabiThume/5623233
I am not sure about it because I am not getting the same error of broken link, so I made a comparison between the current link and "​http://127.0.0.1:6543/auth/forgot_password/?username". Is that right?

[rodney757]

So the reason it is passing is because this isn't a bug in firefox. It is a bug in chrome and safari, maybe others.

With your test:

1. I think it would be best to go directly to /auth/login/.

2. There are at least 2 ways to go about this:

1. Click on the link and test that the status code is 200. I'm not sure how easy it is to do this with selenium.

2. Fill in the username in the login form, and then click the forgot password link. Then:

assert driver.current_url == u'http://127.0.0.1:6543/auth/forgot_password/%3Fusername=user'

where user is the username you entered into the login form.

[Jakob Kramer]

No, that you can put a HREF into window.location.pathname and it puts you to that HREF is a bug in Firefox. The other browsers behave correctly.

[Gabriela Thumé]

I updated the test in ​https://gist.github.com/GabiThume/5623233. Now Firefox passes and Chrome not, as expected.

Unfortunately Selenium does not support http request status code. There is a workaround (​https://coderwall.com/p/aob6ha) if we want to get this status.

Should I also test having the login form filled?

BTW I am thinking about creating a function test for each browser, so we can select which one we want to test. What this sounds like?

[rodney757]

That workaround, won't catch the bug because the problem is with the js not the /auth/forgot_password page.

After looking at it again, I don't think it is necessary to actually fill in the login form, but with your test you need to include an '=' at the end of the assert url.

I'm not sure about the try, except thing with the assert statement. Will the test still fail w/o being run with the -rx option?

That's all I can help you with as I'm not really sure where we are going with selenium testing. Either ask someone on irc or change the status of this bug to review if you think that it's ready for code review.

[Christopher Allan Webber]

Moving to 0.4.1

[rodney757]

So LotusEcho is working on functional testing as part of her summer internship, and will create a test for this :)

[Emily O'Leary]

I've made a test for this and modified it to work under our current testing framework. I just need to set something up so that the server can be started temporarily to run these tests if Selenium is installed.

I use pytests skip function to skip the Selenium tests if it is either not installed or it's version is deprecated. I'll create a new branch once I have that portion working well.

[Emily O'Leary]

Here's a feature branch to add selenium testing to GMG as well as a test case for this particular issue:

​https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commits/selenium_integration

[rodney757]

So i didn't really review much of this, I just ran the tests. So the forgot password test should fail right now, since the we haven't implemented the fix. It passes in your test, because you use the firefox driver (see comment:9 and comment:10). Personally, I think it would be good to use both chrome and firefox drivers, but I'm not sure how cwebb feels about this.

Anyways, thanks for doing this.

[Emily O'Leary]

We moved to a new repo for selenium tests (my version: ​https://gitorious.org/mediagoblin-automation/mediagoblin-selenium). I'm going to be writing a quick script so that it runs with all available drivers.

[Christopher Allan Webber]

I just tested the forgot password link and it works fine. I'm not sure what's going on with this bug. I suspect that it's no longer true... feel free to reopen if true.

The functional testing thing is entirely something different. We never set up a permanent functional testing system, but the scaffolding is there, but that's a separate issue.