Code review comment for lp:~canonical-platform-qa/webbrowser-app/autopilot-back-forward

Revision history for this message
Leo Arias (elopio) wrote :

> The changes you made to test_opening_new_page_enables_back_button() and
> test_navigating_back_enables_forward_button() are not semantically equivalent:
>
> 141 - self.assertThat(back_button.enabled,
> Eventually(Equals(True)))
> 143 + self.main_window.go_back()
> 144 + self.assert_home_page_eventually_loaded()
>
> We don’t want to actually go back, we only want to verify that the back button
> is *eventually* enabled (which the go_back() method doesn’t do). And the
> is_back_button_enabled() method doesn’t contemplate a possible delay (that
> would translate into the use of an Eventually() matcher).

I renamed the tests and added waits in the go methods.
The tests pass here, and I've triggered a new jenkins run to confirm that.

« Back to merge proposal