Repository: wagtail/wagtail
File: wagtail/contrib/simple_translation/tests/test_wagtail_hooks.py (Python)

Code before:
    def unpublish_hook(self, pages, action):
        assert action == "unpublish"
        assert isinstance(pages, list)

    def missing_hook_action(self, pages, action):
        assert action == ""
        assert isinstance(pages, list)

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True)
    def test_page_tree_sync_on(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.unpublish_hook
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "unpublish")
                if response:
                    assert isinstance(response, dict)
                    assert len(response.items()) == 1

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=False)
    def test_page_tree_sync_off(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.unpublish_hook
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "unpublish")
                assert response is None

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True)
    def test_missing_hook_action(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.missing_hook_action
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "")
                if response is not None:
                    assert isinstance(response, dict)

    @override_settings(
        WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True, WAGTAIL_I18N_ENABLED=True
    )
    def test_other_l10n_pages_were_unpublished(self):
        # Login to access the admin
        self.login()

        # Make sur the French homepage is published/live
        self.fr_homepage.live = True
        self.fr_homepage.save()
        assert self.fr_homepage.live is True
        assert self.en_homepage.live is True


Review comment (suggestion):
so.. this is fine, but best to add a note and another assertion

```
defined_hooks = hooks.get_hooks("construct_synced_page_tree_list")
for fn in defined_hooks:
    # We should have two implementations. 
    # One in simple_tranalation.wagtail_hook and the other registered above
    self.assertEqual(len(defined_hooks), 2)

    response = fn([self.en_homepage], "unpublish")
    if response:
        # We 

        # ...
```

aside from that, I'd change the asserts in `test_` methods to the relevant TestCase ones. e.g. `self.assertIsInstance()`, `self.assertEqual()`, `self.assertTrue()`

Code after:
    def unpublish_hook(self, pages, action):
        self.assertEqual(action, "unpublish")
        self.assertIsInstance(pages, list)

    def missing_hook_action(self, pages, action):
        self.assertEqual(action, "")
        self.assertIsInstance(pages, list)

    def test_double_registered_hook(self):
        # We should have two implementations of `construct_synced_page_tree_list`
        # One in simple_translation.wagtail_hooks and the other will be
        # registered as a temporary hook.
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.unpublish_hook
        ):
            defined_hooks = hooks.get_hooks("construct_synced_page_tree_list")
            self.assertEqual(len(defined_hooks), 2)

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True)
    def test_page_tree_sync_on(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.unpublish_hook
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "unpublish")
                if response:
                    self.assertIsInstance(response, dict)
                    self.assertEqual(len(response.items()), 1)

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=False)
    def test_page_tree_sync_off(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.unpublish_hook
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "unpublish")
                self.assertIsNone(response)

    @override_settings(WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True)
    def test_missing_hook_action(self):
        with hooks.register_temporarily(
            "construct_synced_page_tree_list", self.missing_hook_action
        ):
            for fn in hooks.get_hooks("construct_synced_page_tree_list"):
                response = fn([self.en_homepage], "")
                if response is not None:
                    self.assertIsInstance(response, dict)

    @override_settings(
        WAGTAILSIMPLETRANSLATION_SYNC_PAGE_TREE=True, WAGTAIL_I18N_ENABLED=True
    )