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

<matiasb> elopio: on the first one, l.41, wondering if you can't know the url prefix at that point, instead of allowing anything as a url_path prefix (I guess url_path should be the path from /, but there may be a url suffix that matches a suffix of another url? or a login redirect, with a next via query string?)
<elopio> matiasb: I didn't understand the first part, but I think I understood the second.
<elopio> so, do you think it would be better to use urlparse and match just the path
<elopio> ?
<matiasb> elopio: yeah, maybe; wondering if it is worth it, but when I read that assert, I read it as check the url ends with url_path, so checking if that's enough
<elopio> nessita: they do. I'm wondering if it will work setting the info level as default. Would that be ok for you?
<matiasb> elopio: or maybe change the regex to force the match to match starting from the root, if that's not too complicated
<elopio> matiasb: I think it would be easier to get the path from urlparse, and then match the regex there.
<elopio> which makes me wonder if we should add helpers to check query strings and other stuff.
<elopio> but for now, I just want to check the url_path.
<matiasb> yeap, makes sense
<matiasb> elopio: +1 to the add_open_page one
<elopio> \o/
<elopio> matiasb: please take a look at the url_path assertion.
-*- matiasb looks
<matiasb> elopio: I think that would work better; but I think that would still work if current_url_path='/123/testing/test' and url_path='/321/testing/test', right? but maybe that's not a problem, I guess
<matiasb> sorry, url_path='/testing/test'
<elopio> humph, that would be a problem.
<elopio> but easy fixable putting $ at the end of the url_path.
<elopio> matiasb: but the url_path is documented as a regexp. Should I hardcode the $ on the assert_url_path method, or should I define the url_path on the pages with '$'?
<matiasb> elopio: hmm... that makes sense; putting a ^ at the beginning and a $ at the end? (similar to django urls, indeed) I would add that to the url_path definition, to make it more flexible
<matiasb> and in that case you have a re.match instead of a re.search, heh
<elopio> matiasb: ok, but what to you mean with the url_path definition? in the actual page object instances and not in the base class?
<matiasb> (ie, using the regex with ^ and $ for re.search would be really a re.match without the ^ and $)
<elopio> matiasb: ah, yes, you are right.
<matiasb> I think, if it is not too restrictive, having re.match(current_url_path, self.url_path) sounds good
<elopio> but still I don't get what's the best solution for you.
<elopio> no, sounds fine for me. matiasb I'll add a test and change it.
<matiasb> ack, thanks
<matiasb> (it would be the other way round, indeed: re.match(self.url_path, current_url_path), the pattern first)
<elopio> matiasb: pushed. It seems that match takes care only of the ^, so I had to add the $
<matiasb> really? ok, looking
<elopio> matiasb: yes, the suffix test I added was failing.
<matiasb> elopio: ok, looks good :) you just need to escape the + in l.173 and l.227
<elopio> matiasb: good catch. I'll be struggling with that for some time :)
<elopio> pushed.
<matiasb> elopio: you missed one +? l.173 :)
<elopio> agh, I hate this. Sorry matiasb. Fixed and pushed.
<nessita> elopio, cake!
-*- nessita runs away
-*- elopio runs too