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

Revision history for this message
Olivier Tilloy (osomon) 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).

review: Needs Fixing

« Back to merge proposal