Skip to content

test(controller): Add test for controller documentation coverage #2035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 21, 2025

Conversation

karangattu
Copy link
Collaborator

@karangattu karangattu commented Jul 17, 2025

The key changes include enhancements to the documentation configuration, updates to test coverage, and improvements to method implementations for better functionality and maintainability.

Documentation updates:

  • Added new controllers InputBookmarkButton and PageNavbar to the documentation configuration in docs/_quartodoc-testing.yml. This ensures these controllers are now included in the generated documentation.

Functional improvements:

  • Updated the expect_class_state method in shiny/playwright/controller/_output.py to consistently include the timeout parameter in all expect_cell_class calls, improving flexibility and consistency.

Test enhancements:

  • Added assertions in test_validate_row_selection_in_edit_mode to ensure proper row focus and cell state after exiting edit mode, improving the robustness of the test.
  • Introduced a new test file tests/pytest/test_controller_documentation.py to validate that all controller classes are documented and that no extraneous entries exist in the documentation configuration. This ensures alignment between the codebase and the documentation.

Scenario 1: Developer forgets to document it in the testing docs

Code has: Card, Accordion, InputBookmarkButton
Docs have: Card, Accordion

Test fails with:

Controllers missing from docs/_quartodoc-testing.yml:
  - playwright.controller.InputBookmarkButton

Scenario 2: Developer makes a typo in docs

Code has: Card, Accordion, InputBookmarkButton
Docs have: Card, Accordion, InputBookmarkButto

Controllers missing from docs/_quartodoc-testing.yml:
  - playwright.controller.InputBookmarkButton

Extraneous classes in docs/_quartodoc-testing.yml:
  - playwright.controller.InputBookmarkButto

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.
Added InputBookmarkButton to documentation config. Expanded CONTROLLER_BASE_PATTERNS in test_controller_documentation.py to skip additional UI-related base classes.
Deleted the test_documented_classes_format function which checked that documented controller entries are valid Python identifiers.
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.
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.
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.
Deleted an extraneous blank line in the OutputDataFrame class to improve code readability.
Deleted the unnecessary 'timeout' parameter from the test_validate_row_selection_in_edit_mode function call to clean up the test code.
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.
@karangattu karangattu requested a review from schloerke July 17, 2025 14:00
karangattu and others added 3 commits July 17, 2025 21:00
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.
@schloerke schloerke enabled auto-merge (squash) July 21, 2025 18:18
@schloerke schloerke merged commit a9f7807 into main Jul 21, 2025
54 checks passed
@schloerke schloerke deleted the add-bookmark-button-doc branch July 21, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants