Steve, when reviewing browser we try to avoid overriding initialize unless it
is really necessary (I don't entirely recall why), and provide as much as we
can using properties.
Also when writing tests for views there is a very useful test method called:
create_initialized_view
So... with these two thoughts in mind I have the following suggestions:
Also... use create_initialized_view rather than manually creating the review.
view = create_initialized_view(vote, '+reassign')
I also find the flow of the test a little harder to follow with the multiple
login commands. I think it would read slightly better to use
with_person_logged_in (don't forget the future import if it isn't there
already).
def test_view_cancel_url(self):
# Check that the cancel url exists.
bmp = self.factory.makeBranchMergeProposal()
reviewer = self.factory.makePerson()
with person_logged_in(bmp.registrant):
vote = bmp.nominateReviewer( reviewer=reviewer, registrant=bmp.registrant)
with person_logged_in(reviewer):
view = create_initialized_view(vote, '+reassign') self.assertEqual(canonical_url(bmp), view.cancel_url)
A side note: we often use comments instead of docstrings for the test methods
because it isn't always easy to get a single line to describe the test.
Steve, when reviewing browser we try to avoid overriding initialize unless it
is really necessary (I don't entirely recall why), and provide as much as we
can using properties.
Also when writing tests for views there is a very useful test method called: initialized_ view
create_
So... with these two thoughts in mind I have the following suggestions:
@property url(self. context. branch_ merge_proposal)
def next_url(self):
return canonical_
cancel_url = next_url
This means that you don't have to override the initialize method.
Also in the test, you should never have any database IDs, like +merge/1.
Better to assert that the cancel_url matches the canonical_url of the merge
proposal.
self. assertEqual( canonical_ url(bmp) , view.cancel_url)
Also... use create_ initialized_ view rather than manually creating the review.
view = create_ initialized_ view(vote, '+reassign')
I also find the flow of the test a little harder to follow with the multiple logged_ in (don't forget the future import if it isn't there
login commands. I think it would read slightly better to use
with_person_
already).
def test_view_ cancel_ url(self) : makeBranchMerge Proposal( ) makePerson( ) logged_ in(bmp. registrant) : iewer(
reviewer= reviewer, registrant= bmp.registrant) logged_ in(reviewer) : initialized_ view(vote, '+reassign')
self. assertEqual( canonical_ url(bmp) , view.cancel_url)
# Check that the cancel url exists.
bmp = self.factory.
reviewer = self.factory.
with person_
vote = bmp.nominateRev
with person_
view = create_
A side note: we often use comments instead of docstrings for the test methods
because it isn't always easy to get a single line to describe the test.