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

> > It's a way to make sure we are on the right page, and a way to make it
> clearer
> > for the caller where I will be after the call.
>
> In that case I would make it clearer in the method description that you are
> not redirected there but that's where you expect to be after the call (instead
> of 'The page to return on a successful log in.').

You are right, I'll try to improve that.

> > A page instance as a result is what I like too, but I thought the check
> could
> > be done on the action that leads to that page instead of leaving the job to
> > every caller.
>
> Well, right now the caller should know where it would be redirected and pass
> the respective page instance to the action. If that wasn't required it would
> make more sense to me to the action internally check the page is ok, but it
> sounds like the caller is the one who knows where it should be redirected.
>
> > I added to the __init__ a call to assert_page_is_open, and I
> > added it too to all methods that don't instantiate the return page, so
> there's
> > no need for the caller to do an extra check. Maybe I'm not getting right
> what
> > you are saying, if you can give me a little code snippet it will be easier.
>
> I was thinking on something like (pseudo code):
>
> page = sign_in(user)
> self.assert_is_page(page, ExpectedPage) # isinstance?

The problem here is that sign_in doesn't know what's the expected page. If it's called from sso, it would be the account page. If it's called from pay, it would be the payment history page. And from u1, it would be the dashboard.

But what you say is actually an option. It could be something like:

anonymous_home_page = AnonymousHomePage()
ubuntu_single_sign_on = anonymous_home_page.go_to_sign_in()
ubuntu_single_sign_on.sign_in(user)
# Here is a workflow glitch
user_home_page = UserHomePage()
user_home_page.do_the_test()

I think I would prefer:

anonymous_home_page = AnonymousHomePage()
ubuntu_single_sign_on = anonymous_home_page.go_to_sign_in()
user_home_page = ubuntu_single_sign_on.sign_in(user, UserHomePage())
user_home_page.do_the_test()

But I'm definitely not sure about which is best. And maybe both can be improved.

> They look as comments about the attributes and not a class docstring, I think.
>
> This for example (l.331-2):
> title = "{0}'s details"
> """The title of the page. The parameter is the user name."""
> I think should be:
> # The title of the page. The parameter is the user name.
> title = "{0}'s details"

Yes. It was Vincent who told me that the comment was expected after the variable. Any insights Vincent?