From dd66a04cb92fb1a230689c08865fdfb8dba9307b Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 17:02:39 +0530 Subject: [PATCH 01/12] Add test for controller documentation coverage Introduces a pytest test to ensure all controller classes in shiny/playwright/controller are documented in docs/_quartodoc-testing.yml. Also updates the documentation config to include PageNavbar which was missing and the tests caught that. --- docs/_quartodoc-testing.yml | 1 + tests/pytest/test_controller_documentation.py | 147 ++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 tests/pytest/test_controller_documentation.py diff --git a/docs/_quartodoc-testing.yml b/docs/_quartodoc-testing.yml index 8c356c134..f1d7ff930 100644 --- a/docs/_quartodoc-testing.yml +++ b/docs/_quartodoc-testing.yml @@ -59,6 +59,7 @@ quartodoc: - playwright.controller.NavsetTab - playwright.controller.NavsetUnderline - playwright.controller.NavPanel + - playwright.controller.PageNavbar - title: Upload and download desc: Methods for interacting with Shiny app uploading and downloading controller. contents: diff --git a/tests/pytest/test_controller_documentation.py b/tests/pytest/test_controller_documentation.py new file mode 100644 index 000000000..468727f55 --- /dev/null +++ b/tests/pytest/test_controller_documentation.py @@ -0,0 +1,147 @@ +import ast +from pathlib import Path +from typing import Set + +import pytest +import yaml + + +class ControllerDocumentationChecker: + """Validates that all controller classes are documented.""" + + CONTROLLER_DIR = Path("shiny/playwright/controller") + DOCS_CONFIG = Path("docs/_quartodoc-testing.yml") + + SKIP_PATTERNS = {"Base", "Container", "Label", "StyleM"} + CONTROLLER_BASE_PATTERNS = {"Base", "Container", "Label", "StyleM", "WidthLocM"} + + def get_controller_classes(self) -> Set[str]: + """Extract public controller class names from the controller directory.""" + controller_classes: Set[str] = set() + + for py_file in self.CONTROLLER_DIR.glob("*.py"): + if py_file.name == "__init__.py": + continue + + try: + controller_classes.update(self._extract_classes_from_file(py_file)) + except Exception as e: + pytest.fail(f"Failed to parse {py_file}: {e}") + + return controller_classes + + def _extract_classes_from_file(self, py_file: Path) -> Set[str]: + """Extract controller classes from a Python file.""" + with open(py_file, encoding="utf-8") as f: + tree = ast.parse(f.read()) + + classes: Set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef): + if self._is_controller_class(node): + classes.add(node.name) + + return classes + + def _is_controller_class(self, node: ast.ClassDef) -> bool: + """Check if a class definition represents a controller.""" + class_name = node.name + + if class_name.startswith("_"): + return False + + if any(pattern in class_name for pattern in self.SKIP_PATTERNS): + return False + + if self._is_protocol_class(node): + return False + + return self._has_controller_base(node) + + def _is_protocol_class(self, node: ast.ClassDef) -> bool: + """Check if class inherits from a Protocol.""" + return any( + isinstance(base, ast.Name) and base.id.endswith("P") for base in node.bases + ) + + def _has_controller_base(self, node: ast.ClassDef) -> bool: + """Check if class has controller base classes.""" + for base in node.bases: + base_str = ast.unparse(base) + + if base_str.startswith("_"): + return True + + if any(pattern in base_str for pattern in self.CONTROLLER_BASE_PATTERNS): + return True + + return False + + def get_documented_controllers(self) -> Set[str]: + """Extract controller class names from documentation config.""" + try: + with open(self.DOCS_CONFIG, encoding="utf-8") as f: + config = yaml.safe_load(f) + except FileNotFoundError: + pytest.fail(f"Documentation config not found: {self.DOCS_CONFIG}") + except Exception as e: + pytest.fail(f"Failed to parse {self.DOCS_CONFIG}: {e}") + + documented_controllers: Set[str] = set() + + for section in config.get("quartodoc", {}).get("sections", []): + for content in section.get("contents", []): + if isinstance(content, str) and content.startswith( + "playwright.controller." + ): + class_name = content.split(".")[-1] + documented_controllers.add(class_name) + + return documented_controllers + + +@pytest.fixture +def checker(): + """Provide a ControllerDocumentationChecker instance.""" + return ControllerDocumentationChecker() + + +def test_all_controllers_are_documented(checker: ControllerDocumentationChecker): + """Verify all controller classes have documentation entries.""" + controller_classes = checker.get_controller_classes() + documented_controllers = checker.get_documented_controllers() + + missing_controllers = controller_classes - documented_controllers + extra_documented = documented_controllers - controller_classes + + error_messages: list[str] = [] + + if missing_controllers: + missing_list = "\n".join( + f" - playwright.controller.{cls}" for cls in sorted(missing_controllers) + ) + error_messages.append( + f"Controller classes missing from {checker.DOCS_CONFIG}:\n{missing_list}" + ) + + if extra_documented: + extra_list = "\n".join( + f" - playwright.controller.{cls}" for cls in sorted(extra_documented) + ) + error_messages.append(f"Classes documented but not found:\n{extra_list}") + + if error_messages: + pytest.fail("\n\n".join(error_messages)) + + assert controller_classes, "No controller classes found" + assert documented_controllers, "No documented controllers found" + + +def test_documented_classes_format(checker: ControllerDocumentationChecker): + """Verify documented controller entries are valid Python identifiers.""" + documented_controllers = checker.get_documented_controllers() + + invalid_names = [name for name in documented_controllers if not name.isidentifier()] + + if invalid_names: + pytest.fail(f"Invalid controller class names: {invalid_names}") From 07dd42f6e5bb5d3ad9cec6b5a061c4f1739c53be Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 17:14:50 +0530 Subject: [PATCH 02/12] Update docs and tests for new controller classes Added InputBookmarkButton to documentation config. Expanded CONTROLLER_BASE_PATTERNS in test_controller_documentation.py to skip additional UI-related base classes. --- docs/_quartodoc-testing.yml | 1 + tests/pytest/test_controller_documentation.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/_quartodoc-testing.yml b/docs/_quartodoc-testing.yml index f1d7ff930..84de2ef27 100644 --- a/docs/_quartodoc-testing.yml +++ b/docs/_quartodoc-testing.yml @@ -26,6 +26,7 @@ quartodoc: contents: - playwright.controller.InputActionButton - playwright.controller.InputActionLink + - playwright.controller.InputBookmarkButton - playwright.controller.InputCheckbox - playwright.controller.InputCheckboxGroup - playwright.controller.InputDarkMode diff --git a/tests/pytest/test_controller_documentation.py b/tests/pytest/test_controller_documentation.py index 468727f55..7ea289a12 100644 --- a/tests/pytest/test_controller_documentation.py +++ b/tests/pytest/test_controller_documentation.py @@ -13,7 +13,17 @@ class ControllerDocumentationChecker: DOCS_CONFIG = Path("docs/_quartodoc-testing.yml") SKIP_PATTERNS = {"Base", "Container", "Label", "StyleM"} - CONTROLLER_BASE_PATTERNS = {"Base", "Container", "Label", "StyleM", "WidthLocM"} + CONTROLLER_BASE_PATTERNS = { + "Base", + "Container", + "Label", + "StyleM", + "WidthLocM", + "InputActionButton", + "UiBase", + "UiWithLabel", + "UiWithContainer", + } def get_controller_classes(self) -> Set[str]: """Extract public controller class names from the controller directory.""" From f5c74d53d6c3b06d8ef08dac853434658d6e1245 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 17:32:03 +0530 Subject: [PATCH 03/12] Remove test for documented class name format Deleted the test_documented_classes_format function which checked that documented controller entries are valid Python identifiers. --- tests/pytest/test_controller_documentation.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/pytest/test_controller_documentation.py b/tests/pytest/test_controller_documentation.py index 7ea289a12..857b815f2 100644 --- a/tests/pytest/test_controller_documentation.py +++ b/tests/pytest/test_controller_documentation.py @@ -145,13 +145,3 @@ def test_all_controllers_are_documented(checker: ControllerDocumentationChecker) assert controller_classes, "No controller classes found" assert documented_controllers, "No documented controllers found" - - -def test_documented_classes_format(checker: ControllerDocumentationChecker): - """Verify documented controller entries are valid Python identifiers.""" - documented_controllers = checker.get_documented_controllers() - - invalid_names = [name for name in documented_controllers if not name.isidentifier()] - - if invalid_names: - pytest.fail(f"Invalid controller class names: {invalid_names}") From c4c2b48f65d5a631026dd810a5a03fd7d8e92761 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 17:38:07 +0530 Subject: [PATCH 04/12] Refactor controller documentation test logic Replaces the ControllerDocumentationChecker class with standalone functions for extracting controller classes and documented controllers. Simplifies the test structure and error reporting, improving readability and maintainability. --- tests/pytest/test_controller_documentation.py | 185 +++++++----------- 1 file changed, 68 insertions(+), 117 deletions(-) diff --git a/tests/pytest/test_controller_documentation.py b/tests/pytest/test_controller_documentation.py index 857b815f2..5855a473d 100644 --- a/tests/pytest/test_controller_documentation.py +++ b/tests/pytest/test_controller_documentation.py @@ -5,143 +5,94 @@ import pytest import yaml - -class ControllerDocumentationChecker: - """Validates that all controller classes are documented.""" - - CONTROLLER_DIR = Path("shiny/playwright/controller") - DOCS_CONFIG = Path("docs/_quartodoc-testing.yml") - - SKIP_PATTERNS = {"Base", "Container", "Label", "StyleM"} - CONTROLLER_BASE_PATTERNS = { - "Base", - "Container", - "Label", - "StyleM", - "WidthLocM", - "InputActionButton", - "UiBase", - "UiWithLabel", - "UiWithContainer", - } - - def get_controller_classes(self) -> Set[str]: - """Extract public controller class names from the controller directory.""" - controller_classes: Set[str] = set() - - for py_file in self.CONTROLLER_DIR.glob("*.py"): - if py_file.name == "__init__.py": - continue - - try: - controller_classes.update(self._extract_classes_from_file(py_file)) - except Exception as e: - pytest.fail(f"Failed to parse {py_file}: {e}") - - return controller_classes - - def _extract_classes_from_file(self, py_file: Path) -> Set[str]: - """Extract controller classes from a Python file.""" - with open(py_file, encoding="utf-8") as f: - tree = ast.parse(f.read()) - - classes: Set[str] = set() - for node in ast.walk(tree): - if isinstance(node, ast.ClassDef): - if self._is_controller_class(node): - classes.add(node.name) - - return classes - - def _is_controller_class(self, node: ast.ClassDef) -> bool: - """Check if a class definition represents a controller.""" - class_name = node.name - - if class_name.startswith("_"): - return False - - if any(pattern in class_name for pattern in self.SKIP_PATTERNS): - return False - - if self._is_protocol_class(node): - return False - - return self._has_controller_base(node) - - def _is_protocol_class(self, node: ast.ClassDef) -> bool: - """Check if class inherits from a Protocol.""" - return any( - isinstance(base, ast.Name) and base.id.endswith("P") for base in node.bases +CONTROLLER_DIR = Path("shiny/playwright/controller") +DOCS_CONFIG = Path("docs/_quartodoc-testing.yml") +SKIP_PATTERNS = {"Base", "Container", "Label", "StyleM"} +CONTROLLER_BASE_PATTERNS = { + "Base", + "Container", + "Label", + "StyleM", + "WidthLocM", + "InputActionButton", + "UiBase", + "UiWithLabel", + "UiWithContainer", +} + + +def _is_valid_controller_class(node: ast.ClassDef) -> bool: + class_name = node.name + base_names = {ast.unparse(base) for base in node.bases} + + return ( + not class_name.startswith("_") + and not any(pattern in class_name for pattern in SKIP_PATTERNS) + and not any(base.endswith("P") for base in base_names if isinstance(base, str)) + and any( + base.startswith("_") or any(p in base for p in CONTROLLER_BASE_PATTERNS) + for base in base_names ) + ) - def _has_controller_base(self, node: ast.ClassDef) -> bool: - """Check if class has controller base classes.""" - for base in node.bases: - base_str = ast.unparse(base) - - if base_str.startswith("_"): - return True - if any(pattern in base_str for pattern in self.CONTROLLER_BASE_PATTERNS): - return True - - return False - - def get_documented_controllers(self) -> Set[str]: - """Extract controller class names from documentation config.""" +def get_controller_classes() -> Set[str]: + classes: Set[str] = set() + for py_file in CONTROLLER_DIR.glob("*.py"): + if py_file.name == "__init__.py": + continue try: - with open(self.DOCS_CONFIG, encoding="utf-8") as f: - config = yaml.safe_load(f) - except FileNotFoundError: - pytest.fail(f"Documentation config not found: {self.DOCS_CONFIG}") + tree = ast.parse(py_file.read_text(encoding="utf-8")) + classes.update( + node.name + for node in ast.walk(tree) + if isinstance(node, ast.ClassDef) and _is_valid_controller_class(node) + ) except Exception as e: - pytest.fail(f"Failed to parse {self.DOCS_CONFIG}: {e}") - - documented_controllers: Set[str] = set() + pytest.fail(f"Failed to parse {py_file}: {e}") + return classes - for section in config.get("quartodoc", {}).get("sections", []): - for content in section.get("contents", []): - if isinstance(content, str) and content.startswith( - "playwright.controller." - ): - class_name = content.split(".")[-1] - documented_controllers.add(class_name) - return documented_controllers +def get_documented_controllers() -> Set[str]: + try: + config = yaml.safe_load(DOCS_CONFIG.read_text(encoding="utf-8")) + except Exception as e: + pytest.fail(f"Failed to load or parse {DOCS_CONFIG}: {e}") - -@pytest.fixture -def checker(): - """Provide a ControllerDocumentationChecker instance.""" - return ControllerDocumentationChecker() + return { + content.split(".")[-1] + for section in config.get("quartodoc", {}).get("sections", []) + for content in section.get("contents", []) + if isinstance(content, str) and content.startswith("playwright.controller.") + } -def test_all_controllers_are_documented(checker: ControllerDocumentationChecker): - """Verify all controller classes have documentation entries.""" - controller_classes = checker.get_controller_classes() - documented_controllers = checker.get_documented_controllers() +def test_all_controllers_are_documented(): + controller_classes = get_controller_classes() + documented_controllers = get_documented_controllers() - missing_controllers = controller_classes - documented_controllers - extra_documented = documented_controllers - controller_classes + missing_from_docs = controller_classes - documented_controllers + extra_in_docs = documented_controllers - controller_classes - error_messages: list[str] = [] + from typing import List - if missing_controllers: + error_messages: List[str] = [] + if missing_from_docs: missing_list = "\n".join( - f" - playwright.controller.{cls}" for cls in sorted(missing_controllers) + sorted(f" - playwright.controller.{c}" for c in missing_from_docs) ) error_messages.append( - f"Controller classes missing from {checker.DOCS_CONFIG}:\n{missing_list}" + f"Controllers missing from {DOCS_CONFIG}:\n{missing_list}" ) - if extra_documented: + if extra_in_docs: extra_list = "\n".join( - f" - playwright.controller.{cls}" for cls in sorted(extra_documented) + sorted(f" - playwright.controller.{c}" for c in extra_in_docs) ) - error_messages.append(f"Classes documented but not found:\n{extra_list}") + error_messages.append(f"Extraneous classes in {DOCS_CONFIG}:\n{extra_list}") if error_messages: - pytest.fail("\n\n".join(error_messages)) + pytest.fail("\n\n".join(error_messages), pytrace=False) - assert controller_classes, "No controller classes found" - assert documented_controllers, "No documented controllers found" + assert controller_classes, "No controller classes were found." + assert documented_controllers, "No documented controllers were found." From 863ea5433513110d390c5284b98c99b63ee199b2 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 18:13:42 +0530 Subject: [PATCH 05/12] Add timeout handling for cell editing state checks Updated OutputDataFrame to pass timeout to expect_cell_class for all cell state checks. Improved test reliability by adding retries and timeout when verifying cell enters editing state after pressing Enter. --- shiny/playwright/controller/_output.py | 19 +++++++++++---- .../test_validate_row_selection_edit_mode.py | 23 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/shiny/playwright/controller/_output.py b/shiny/playwright/controller/_output.py index 614407b83..c35aee476 100644 --- a/shiny/playwright/controller/_output.py +++ b/shiny/playwright/controller/_output.py @@ -1058,13 +1058,21 @@ def expect_class_state( "cell-edit-editing", timeout=timeout ) elif value == "editing": - self.expect_cell_class("cell-edit-editing", row=row, col=col) + self.expect_cell_class( + "cell-edit-editing", row=row, col=col, timeout=timeout + ) elif value == "saving": - self.expect_cell_class("cell-edit-saving", row=row, col=col) + self.expect_cell_class( + "cell-edit-saving", row=row, col=col, timeout=timeout + ) elif value == "failure": - self.expect_cell_class("cell-edit-failure", row=row, col=col) + self.expect_cell_class( + "cell-edit-failure", row=row, col=col, timeout=timeout + ) elif value == "success": - self.expect_cell_class("cell-edit-success", row=row, col=col) + self.expect_cell_class( + "cell-edit-success", row=row, col=col, timeout=timeout + ) else: raise ValueError( "Invalid state. Select one of 'success', 'failure', 'saving', 'editing', 'ready'" @@ -1096,6 +1104,9 @@ def _edit_cell_no_save( self._cell_scroll_if_needed(row=row, col=col, timeout=timeout) cell.dblclick(timeout=timeout) + + # Wait for the cell to enter editing mode before filling the textarea + expect_to_have_class(cell, "cell-edit-editing", timeout=timeout) cell.locator("> textarea").fill(text) def set_sort( diff --git a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py index a154576bd..60ca85a55 100644 --- a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py +++ b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py @@ -61,11 +61,24 @@ def test_validate_row_selection_in_edit_mode( data_frame._edit_cell_no_save("Temp value", row=1, col=16) page.keyboard.press("Escape") page.keyboard.press("Enter") - data_frame.expect_class_state( - "editing", - row=1, - col=0, - ) # Stage column begins to be edited. + + # This interaction (Enter key to start editing) can be flaky, so we retry if needed + max_retries = 3 + for attempt in range(max_retries): + try: + page.wait_for_timeout(100) + data_frame.expect_class_state( + "editing", + row=1, + col=0, + timeout=1000, + ) + break + except AssertionError: + if attempt < max_retries - 1: + page.keyboard.press("Enter") + else: + raise # Click outside the table/Press Escape to exit row focus. # Tab to the column name, hit enter. Verify the table becomes sorted. From 5191764e4662c08c35f4173bf247a7c3f6b59844 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 18:46:37 +0530 Subject: [PATCH 06/12] Simplify cell editing logic in DataFrame output and test Removed unnecessary class expectation when editing cells in OutputDataFrame and simplified flaky retry logic in the related test by using a single wait and class state check. This streamlines cell editing interactions and improves test reliability. --- shiny/playwright/controller/_output.py | 2 -- .../test_validate_row_selection_edit_mode.py | 26 ++++++------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/shiny/playwright/controller/_output.py b/shiny/playwright/controller/_output.py index c35aee476..61e0ed543 100644 --- a/shiny/playwright/controller/_output.py +++ b/shiny/playwright/controller/_output.py @@ -1105,8 +1105,6 @@ def _edit_cell_no_save( self._cell_scroll_if_needed(row=row, col=col, timeout=timeout) cell.dblclick(timeout=timeout) - # Wait for the cell to enter editing mode before filling the textarea - expect_to_have_class(cell, "cell-edit-editing", timeout=timeout) cell.locator("> textarea").fill(text) def set_sort( diff --git a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py index 60ca85a55..7e54263cb 100644 --- a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py +++ b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py @@ -60,25 +60,15 @@ def test_validate_row_selection_in_edit_mode( page.keyboard.press("Escape") data_frame._edit_cell_no_save("Temp value", row=1, col=16) page.keyboard.press("Escape") + # Add a small delay + page.wait_for_timeout(100) page.keyboard.press("Enter") - - # This interaction (Enter key to start editing) can be flaky, so we retry if needed - max_retries = 3 - for attempt in range(max_retries): - try: - page.wait_for_timeout(100) - data_frame.expect_class_state( - "editing", - row=1, - col=0, - timeout=1000, - ) - break - except AssertionError: - if attempt < max_retries - 1: - page.keyboard.press("Enter") - else: - raise + data_frame.expect_class_state( + "editing", + row=1, + col=0, + timeout=1000, + ) # Click outside the table/Press Escape to exit row focus. # Tab to the column name, hit enter. Verify the table becomes sorted. From 5bbb4ed44f1d70d42db36bf1d710f6c4ada19664 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 18:49:07 +0530 Subject: [PATCH 07/12] Remove unnecessary blank line in OutputDataFrame Deleted an extraneous blank line in the OutputDataFrame class to improve code readability. --- shiny/playwright/controller/_output.py | 1 - 1 file changed, 1 deletion(-) diff --git a/shiny/playwright/controller/_output.py b/shiny/playwright/controller/_output.py index 61e0ed543..416313657 100644 --- a/shiny/playwright/controller/_output.py +++ b/shiny/playwright/controller/_output.py @@ -1104,7 +1104,6 @@ def _edit_cell_no_save( self._cell_scroll_if_needed(row=row, col=col, timeout=timeout) cell.dblclick(timeout=timeout) - cell.locator("> textarea").fill(text) def set_sort( From 4ae6a6302265c994345283763c31563cb7489ff1 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 18:50:58 +0530 Subject: [PATCH 08/12] Remove unused timeout argument in test Deleted the unnecessary 'timeout' parameter from the test_validate_row_selection_in_edit_mode function call to clean up the test code. --- .../test_validate_row_selection_edit_mode.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py index 7e54263cb..3fc592af6 100644 --- a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py +++ b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py @@ -67,7 +67,6 @@ def test_validate_row_selection_in_edit_mode( "editing", row=1, col=0, - timeout=1000, ) # Click outside the table/Press Escape to exit row focus. From a806e2810e60de5730d0fb46d780a92868e3ef3b Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 18:59:39 +0530 Subject: [PATCH 09/12] Add comment clarifying edit mode test step A comment was added to explain that the stage column begins to be edited in the test_validate_row_selection_in_edit_mode test. This improves code readability for future maintenance. --- .../test_validate_row_selection_edit_mode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py index 3fc592af6..8db18a5d2 100644 --- a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py +++ b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py @@ -67,7 +67,7 @@ def test_validate_row_selection_in_edit_mode( "editing", row=1, col=0, - ) + ) # Stage column begins to be edited. # Click outside the table/Press Escape to exit row focus. # Tab to the column name, hit enter. Verify the table becomes sorted. From 8f648b090f7f546682730c75d41726be3c579c69 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 17 Jul 2025 21:00:46 +0530 Subject: [PATCH 10/12] Improve row selection validation in edit mode test Replaced a fixed timeout with explicit checks for row focus and cell editing state after escaping edit mode. This makes the test more robust and less dependent on timing. --- .../test_validate_row_selection_edit_mode.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py index 8db18a5d2..823bb33f6 100644 --- a/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py +++ b/tests/playwright/shiny/components/data_frame/validate_row_selection_edit_mode/test_validate_row_selection_edit_mode.py @@ -60,8 +60,10 @@ def test_validate_row_selection_in_edit_mode( page.keyboard.press("Escape") data_frame._edit_cell_no_save("Temp value", row=1, col=16) page.keyboard.press("Escape") - # Add a small delay - page.wait_for_timeout(100) + # Wait for the row to be focused again after escaping edit mode + data_frame._expect_row_focus_state(True, row=1) + # Also ensure the cell is no longer in editing state + data_frame.expect_class_state("ready", row=1, col=16) page.keyboard.press("Enter") data_frame.expect_class_state( "editing", From e23962fab14d70189c59e2ef374077f76e18377a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 21 Jul 2025 14:17:25 -0400 Subject: [PATCH 11/12] cosmetic --- shiny/playwright/controller/_output.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/shiny/playwright/controller/_output.py b/shiny/playwright/controller/_output.py index 416313657..ce956385b 100644 --- a/shiny/playwright/controller/_output.py +++ b/shiny/playwright/controller/_output.py @@ -1059,19 +1059,31 @@ def expect_class_state( ) elif value == "editing": self.expect_cell_class( - "cell-edit-editing", row=row, col=col, timeout=timeout + "cell-edit-editing", + row=row, + col=col, + timeout=timeout, ) elif value == "saving": self.expect_cell_class( - "cell-edit-saving", row=row, col=col, timeout=timeout + "cell-edit-saving", + row=row, + col=col, + timeout=timeout, ) elif value == "failure": self.expect_cell_class( - "cell-edit-failure", row=row, col=col, timeout=timeout + "cell-edit-failure", + row=row, + col=col, + timeout=timeout, ) elif value == "success": self.expect_cell_class( - "cell-edit-success", row=row, col=col, timeout=timeout + "cell-edit-success", + row=row, + col=col, + timeout=timeout, ) else: raise ValueError( From e6629a481d90d62276da9f6c9ec6ddd6ce4d8e21 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 21 Jul 2025 14:18:21 -0400 Subject: [PATCH 12/12] cosmetic --- tests/pytest/test_controller_documentation.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/pytest/test_controller_documentation.py b/tests/pytest/test_controller_documentation.py index 5855a473d..f772a0f58 100644 --- a/tests/pytest/test_controller_documentation.py +++ b/tests/pytest/test_controller_documentation.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import ast from pathlib import Path from typing import Set @@ -5,8 +7,10 @@ import pytest import yaml -CONTROLLER_DIR = Path("shiny/playwright/controller") -DOCS_CONFIG = Path("docs/_quartodoc-testing.yml") +root = Path(__file__).parent.parent.parent + +CONTROLLER_DIR = root / "shiny/playwright/controller" +DOCS_CONFIG = root / "docs/_quartodoc-testing.yml" SKIP_PATTERNS = {"Base", "Container", "Label", "StyleM"} CONTROLLER_BASE_PATTERNS = { "Base", @@ -74,9 +78,7 @@ def test_all_controllers_are_documented(): missing_from_docs = controller_classes - documented_controllers extra_in_docs = documented_controllers - controller_classes - from typing import List - - error_messages: List[str] = [] + error_messages: list[str] = [] if missing_from_docs: missing_list = "\n".join( sorted(f" - playwright.controller.{c}" for c in missing_from_docs)