Closed Bug 1386744 Opened 7 years ago Closed 7 years ago

Better Error Messages for test_restore_windows_after_restart_and_quit

Categories

(Testing :: Firefox UI Tests, defect)

defect
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ewright, Assigned: ewright)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

It is difficult to figure out what is actually going wrong with bug 1382154. This is to create better error messages in the tests.
Blocks: 1382453
Comment on attachment 8893006 [details]
Bug 1386744 - Better error messages on test failures.

https://reviewboard.mozilla.org/r/164000/#review169348

::: testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart_and_quit.py:161
(Diff revision 1)
>          restored, and that the private ones have not.
>          """
> -        self.assertEqual(self.convert_open_windows_to_set(), self.all_windows,
> -                         msg='Not all requested windows have been opened.')
> +        current_windows_set = self.convert_open_windows_to_set()
> +        self.assertEqual(current_windows_set, self.all_windows,
> +                         msg='Not all requested windows have been opened. Expected {}, got {}.'
> +                         .format(current_windows_set, self.all_windows))

You mix up expected with got here.

::: testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart_and_quit.py:171
(Diff revision 1)
>  
> -        self.assertEqual(self.convert_open_windows_to_set(), self.test_windows,
> -                         msg='Non private windows and tabs should have been restored.')
> +        current_windows_set = self.convert_open_windows_to_set()
> +        self.assertEqual(current_windows_set, self.test_windows,
> +                         msg="""Non private windows and tabs should
> +                         have been restored. Expected {}, got {}.
> +                         """.format(self.test_windows, current_windows_set))

Hm, can we be sure that all windows and tabs have been opened at this time? Maybe that is the problem on the other bug, and we just need a Wait().until() here. But lets leave that for now.

But what do you actually mean with tabs? Tabs inside of private browsing windows? If yes, it would be enough to only call out the private browsing windows here.

::: testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart_and_quit.py:179
(Diff revision 1)
>  class TestSessionStoreDisabled(TestBaseRestoreWindows):
>      def test_no_restore_with_quit(self):
> -        self.assertEqual(self.convert_open_windows_to_set(), self.all_windows,
> -                         msg='Not all requested windows have been opened.')
> +        current_windows_set = self.convert_open_windows_to_set()
> +        self.assertEqual(current_windows_set, self.all_windows,
> +                         msg='Not all requested windows have been opened. Expected {}, got {}.'
> +                         .format(current_windows_set, self.all_windows))

Same as above.

::: testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart_and_quit.py:193
(Diff revision 1)
>                           msg='Tabs from last session shouldn`t have been restored.')
>  
>      def test_restore_with_restart(self):
> -        self.assertEqual(self.convert_open_windows_to_set(), self.all_windows,
> -                         msg='Not all requested windows have been opened.')
> +        current_windows_set = self.convert_open_windows_to_set()
> +        self.assertEqual(current_windows_set, self.all_windows,
> +                         msg='Not all requested windows have been opened. Expected {}, got {}.'

Same as above.

::: testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart_and_quit.py:202
(Diff revision 1)
>  
> -        self.assertEqual(self.convert_open_windows_to_set(), self.test_windows,
> -                         msg='Non private windows and tabs should have been restored.')
> +        current_windows_set = self.convert_open_windows_to_set()
> +        self.assertEqual(current_windows_set, self.test_windows,
> +                         msg="""Non private windows and tabs should
> +                         have been restored. Expected {}, got {}.
> +                         """.format(self.test_windows, current_windows_set))

Same as above regarding tabs.
Attachment #8893006 - Flags: review?(hskupin) → review-
Comment on attachment 8893006 [details]
Bug 1386744 - Better error messages on test failures.

https://reviewboard.mozilla.org/r/164000/#review169348

> You mix up expected with got here.

I don't see in the lastest update that this has been fixed. Instead you flipped the arguments for other assertions (private browsing window exclusion).

> Hm, can we be sure that all windows and tabs have been opened at this time? Maybe that is the problem on the other bug, and we just need a Wait().until() here. But lets leave that for now.
> 
> But what do you actually mean with tabs? Tabs inside of private browsing windows? If yes, it would be enough to only call out the private browsing windows here.

The wording was fine formerly, and is a bit long now. Why not using "Not all non private browsing windows have been restored...".

Also you wrongly flipped the arguments for `format` here.
Comment on attachment 8893006 [details]
Bug 1386744 - Better error messages on test failures.

https://reviewboard.mozilla.org/r/164000/#review170234
Attachment #8893006 - Flags: review?(hskupin) → review-
Comment on attachment 8893006 [details]
Bug 1386744 - Better error messages on test failures.

https://reviewboard.mozilla.org/r/164000/#review169348

> I don't see in the lastest update that this has been fixed. Instead you flipped the arguments for other assertions (private browsing window exclusion).

You're right, I'm reading my own word incorrectly, sorry
Comment on attachment 8893006 [details]
Bug 1386744 - Better error messages on test failures.

https://reviewboard.mozilla.org/r/164000/#review170732
Attachment #8893006 - Flags: review?(hskupin) → review+
Pushed by bwinton@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0a864923a1b4
Better error messages on test failures. r=whimboo
https://hg.mozilla.org/mozilla-central/rev/0a864923a1b4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: