Fix unit test interdependence (#84)

vizro-core/changelog.d/20231001_000340_huong_li_nguyen_fix_test_interdependence.md

@@ -0,0 +1,41 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+
+### Fixed
+
+- Fix unit test interdependence issue due to shared dash.page_registry ([#84](https://github.com/mckinsey/vizro/pull/84))
+
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

vizro-core/docs/pages/development/authors.md

@@ -14,6 +14,7 @@
 
 ## Previous team members and code contributors
 
+[axa99](https://github.com/axa99),
 [Jo Stichbury](https://github.com/stichbury),
 [Juan Luis Cano Rodríguez](https://github.com/astrojuanlu),
 [Denis Lebedev](https://github.com/DenisLebedevMcK),

vizro-core/src/vizro/_constants.py

@@ -6,4 +6,4 @@
 ON_PAGE_LOAD_ACTION_PREFIX = "on_page_load_action"
 FILTER_ACTION_PREFIX = "filter_action"
 PARAMETER_ACTION_PREFIX = "parameter_action"
-ACCORDION_TITLE = "SELECT PAGE"
+ACCORDION_DEFAULT_TITLE = "SELECT PAGE"

vizro-core/src/vizro/models/_navigation/_accordion.py

@@ -5,7 +5,7 @@
 from dash import html
 from pydantic import validator
 
-from vizro._constants import ACCORDION_TITLE, MODULE_PAGE_404
+from vizro._constants import ACCORDION_DEFAULT_TITLE
 from vizro.models import VizroBaseModel
 from vizro.models._models_utils import _log_call
 from vizro.models._navigation.navigation import _validate_pages
@@ -27,11 +27,13 @@ class Accordion(VizroBaseModel):
 
     @_log_call
     def build(self):
-        if self.pages:
-            return self._create_accordion()
-        return self._create_default_accordion()
+        return self._create_accordion()
 
-    def _create_accordion_buttons(self, accordion_pages):
+    def _create_accordion_buttons(self, pages):
+        """Creates a button for each provided page."""
+        # TODO: Better if we loop through pages from MM so the Accordion.build does not depend on dashboard build.
+        # However, this would require that only pages used in the Dashboard are registered in the MM.
+        # Note: Relative path currently deviates from page.path for first page.
         return [
             dbc.Button(
                 children=[page["name"]],
@@ -40,33 +42,20 @@ def _create_accordion_buttons(self, accordion_pages):
                 href=page["relative_path"],
             )
             for page in dash.page_registry.values()
-            for accordion_page in accordion_pages
-            if accordion_page == page["module"] and page["module"] != MODULE_PAGE_404
+            if page["module"] in pages
         ]
 
-    def _create_accordion_item(self, accordion_buttons, title=ACCORDION_TITLE):
+    def _create_accordion_item(self, accordion_buttons, title=ACCORDION_DEFAULT_TITLE):
+        """Creates an accordion item for each sub-group of pages."""
         return dbc.AccordionItem(
             children=accordion_buttons,
             title=title.upper(),
             class_name="accordion_item",
         )
 
-    def _create_default_accordion(self):
-        accordion_buttons = [
-            dbc.Button(
-                children=[page["name"]],
-                key=page["relative_path"],
-                className="accordion-item-button",
-                href=page["relative_path"],
-            )
-            for page in dash.page_registry.values()
-            if page["module"] != MODULE_PAGE_404
-        ]
-
-        accordion_items = [self._create_accordion_item(accordion_buttons=accordion_buttons)]
-
-        # Don't create accordion navigation if there is only one page and one accordion item
-        if len(accordion_buttons) == len(accordion_items) == 1:
+    def _get_accordion_container(self, accordion_items, accordion_buttons):
+        # Return no container if there is only one page in the dashboard or no pages exist
+        if (len(accordion_buttons) == len(accordion_items) == 1) or not accordion_buttons:
             return None
 
         return html.Div(
@@ -85,31 +74,16 @@ def _create_default_accordion(self):
         )
 
     def _create_accordion(self):
+        """Creates a custom accordion only with user-provided pages."""
         accordion_items = []
-
         if isinstance(self.pages, dict):
-            for title, accordion_pages in self.pages.items():
-                accordion_buttons = self._create_accordion_buttons(accordion_pages=accordion_pages)
-                accordion_items.append(self._create_accordion_item(accordion_buttons=accordion_buttons, title=title))
+            for page_group, page_members in self.pages.items():
+                accordion_buttons = self._create_accordion_buttons(pages=page_members)
+                accordion_items.append(
+                    self._create_accordion_item(accordion_buttons=accordion_buttons, title=page_group)
+                )
 
         if isinstance(self.pages, list):
-            accordion_buttons = self._create_accordion_buttons(accordion_pages=self.pages)
+            accordion_buttons = self._create_accordion_buttons(pages=self.pages)
             accordion_items.append(self._create_accordion_item(accordion_buttons=accordion_buttons))
-
-        if len(accordion_buttons) == len(accordion_items) == 1:
-            return None
-
-        return html.Div(
-            children=[
-                dbc.Accordion(
-                    id=self.id,
-                    children=accordion_items,
-                    class_name="accordion",
-                    persistence=True,
-                    persistence_type="session",
-                ),
-                html.Div(className="keyline"),
-            ],
-            className="nav_panel",
-            id=f"{self.id}_outer",
-        )
+        return self._get_accordion_container(accordion_items=accordion_items, accordion_buttons=accordion_buttons)

vizro-core/src/vizro/models/_navigation/navigation.py

@@ -3,8 +3,10 @@
 import warnings
 from typing import TYPE_CHECKING, Optional
 
+import dash
 from pydantic import PrivateAttr, validator
 
+from vizro._constants import MODULE_PAGE_404
 from vizro.managers import model_manager
 from vizro.models import VizroBaseModel
 from vizro.models._models_utils import _log_call
@@ -45,6 +47,9 @@ def _validate_pages(pages):
                 f"Unknown page ID or page title provided to Navigation 'pages'. " f"Unknown pages: {unknown_pages}"
             )
 
+    if pages is None:
+        return [page for page in dash.page_registry.keys() if page != MODULE_PAGE_404]
+
     return pages
 
 

vizro-core/tests/unit/vizro/conftest.py

@@ -1,4 +1,6 @@
 """Fixtures to be shared across several tests."""
+
+import dash
 import pytest
 
 import vizro.models as vm
@@ -24,15 +26,23 @@ def standard_px_chart(gapminder):
 
 
 @pytest.fixture()
-def two_pages():
-    two_pages = [
-        vm.Page(title="Page 1", components=[vm.Button(), vm.Button()]),
-        vm.Page(title="Page 2", components=[vm.Button(), vm.Button()]),
-    ]
-    return two_pages
+def page1():
+    return vm.Page(title="Page 1", components=[vm.Button(), vm.Button()])
+
+
+@pytest.fixture()
+def page2():
+    return vm.Page(title="Page 2", components=[vm.Button(), vm.Button()])
 
 
 @pytest.fixture()
-def dashboard(two_pages):
-    dashboard = vm.Dashboard(pages=two_pages)
+def dashboard(page1, page2):
+    dashboard = vm.Dashboard(pages=[page1, page2])
     return dashboard
+
+
+@pytest.fixture()
+def dashboard_build(dashboard):
+    yield dashboard.build()
+    del dash.page_registry["Page 1"]
+    del dash.page_registry["Page 2"]

vizro-core/tests/unit/vizro/models/_navigation/conftest.py

@@ -1,14 +1,10 @@
 """Fixtures to be shared across several tests."""
+
 import dash_bootstrap_components as dbc
 import pytest
 from dash import html
 
-from vizro import Vizro
-
-
-@pytest.fixture
-def dashboard_build(dashboard):
-    return Vizro().build(dashboard)
+from vizro._constants import ACCORDION_DEFAULT_TITLE
 
 
 @pytest.fixture()
@@ -40,7 +36,7 @@ def accordion_from_page_as_list():
     accordion_items = [
         dbc.AccordionItem(
             children=[*accordion_buttons],
-            title="SELECT PAGE",
+            title=ACCORDION_DEFAULT_TITLE,
             class_name="accordion_item",
         )
     ]

vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py

@@ -16,7 +16,7 @@ class TestAccordionInstantiation:
     def test_create_accordion_default(self):
         accordion = Accordion(id="accordion_id")
         assert accordion.id == "accordion_id"
-        assert accordion.pages is None
+        assert accordion.pages == ["Page 1", "Page 2"]
 
     def test_create_accordion_pages_as_list(self, pages_as_list):
         accordion = Accordion(pages=pages_as_list, id="accordion_id")
@@ -64,6 +64,6 @@ def test_accordion_build_single_page_accordion(self):
         accordion = Accordion(pages=["Page 1"], id="single_accordion").build()
         assert accordion is None
 
-    def test_navigation_not_all_pages_included(self, dashboard_build):
+    def test_navigation_not_all_pages_included(self):
         with pytest.warns(UserWarning):
             Accordion(pages=["Page 1"])

vizro-core/tests/unit/vizro/models/_navigation/test_navigation.py

@@ -16,7 +16,7 @@ class TestNavigationInstantiation:
     def test_navigation_default(self):
         navigation = vm.Navigation(id="navigation")
         assert navigation.id == "navigation"
-        assert navigation.pages is None
+        assert navigation.pages == ["Page 1", "Page 2"]
 
     def test_navigation_pages_as_list(self, pages_as_list):
         navigation = vm.Navigation(pages=pages_as_list, id="navigation")
@@ -28,7 +28,7 @@ def test_navigation_pages_as_dict(self, pages_as_dict):
         assert navigation.id == "navigation"
         assert navigation.pages == pages_as_dict
 
-    def test_navigation_not_all_pages_included(self, dashboard_build):
+    def test_navigation_not_all_pages_included(self):
         with pytest.warns(UserWarning):
             vm.Navigation(pages=["Page 1"])