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.').

> 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?

instead of:

expected_page = ExpectedPage()
page = sign_in(user, return_page=expected_page)

But this is just an opinion, maybe I'm missing something/context.

> I think not. Why? They are docstrings, and according to pep257 they should use
> triple double quotes. It really doesn't say anything specific to class
> attributes, but that's what I understood. I might be wrong.

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"