Code review comment for lp:~elopio/u1-test-utils/log_in_from_redirect

Vila, Julien,

So, after a loooong discussion, we came up with the possible solutions.

I implemented two, so you can take a look and compare them on a better
way. Let me make a summary

1. Do nothing:

This is not the same as leaving untested code. This means that all the
code paths for the create_new_account helper in u1testutils are
exercised by the test_register_new_account in the pay project. If
something is wrong on the helper or if the code in pay changes, we will
notice immediately because we will be having a test failing. To fix
something in this helper means making this test pass.

Of course, I'm still in favor of this solution.

The problem is that the test is in a different project than the code.

2. Add a test without a server:

This was what you proposed at the end of the meeting. The only way (I
can find) of testing the fix without a fake or real server is mocking
the actions and just checking they were called.

http://paste.ubuntu.com/5619597/

The test that's there will fail until we merge the fix I prepared on the
original MP.
The problem is that it's mocking almost everything, thus the value of
the test is small.

3. Add a test with a fake server:

I think this is what Vincent likes. I tried to implement it with fake
html pages:

http://paste.ubuntu.com/5619829/

There are still some rough details as it's just a prototype, but the
idea is there.
It took me like one hour to make the test, so the reason I don't like
this solution is not because it's hard or I'm lazy.

I don't like it because if a dev wants to change the heding1 that says:
are you new?, now he has to change the original html, the page object
and the fake we have in this test.
It's even worse if he wants to change the flow or registration (as they
are doing with the new theme). Then a change would be required in the
html templates, in the tests for those templates, in the pay acceptance
test_register_new_user, in the u1testutils helpers, in the u1testutils
page object and in the faked page added by this test.
I would appreciated having one less change to do.

Also I don't like it because this is pretty simple to do; but we also
have helpers for the log in, for complete a payment and for changing the
payment preferences. Then we will end with many more than two faked
htmlstringpages.

4. Add a test with a real server:

The test would be copying test_register_new_user from pay to
u1testutils. It will look a lot like the test in (3), but instead of
opening an string page, we would open either a local SSO or staging SSO.
Currently starting a local SSO takes a lot of time, so I don't like that
way.
And I liked the idea of staging SSO, I started using that for the API
helpers, but failed repeatedly as staging SSO was changed or being
updated. So now I don't like it.

===============================

I think that the penalty for doing (1) it's not big, and I consider
(2-4) to be duplicating the same thing we are testing on
test_register_new_user, adding a new layer we need to maintain, and with
small added value.

That's my opinion, I rest my case. I'm showing here I can implement any
one of the solutions, and as what I want now is just to make pay tests
to go green as soon as possible, I'll do whatever you two decide,
without hard feelings

pura vida.