docs: add Playwright page object design conventions - #1154
Open
mrrajan wants to merge 1 commit into
Open
Conversation
Document two E2E testing conventions identified during TC-3811 verification: no explicit timeouts in shared page objects, and composition over conditional expansion in page object methods. Implements TC-5191 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Assisted-by: Claude Code
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates testing conventions documentation to add two Playwright page object design rules around timeouts and method responsibilities. Flow diagram for Playwright page object design conventionsflowchart TD
PageObjectMethod["Designing a Playwright page object method"]
PageObjectMethod --> TimeoutDecision["Need to wait for async UI change?"]
TimeoutDecision -->|Yes| SpecificPageObjectWait["Add wait in specific page object (e.g., SbomListPage)"]
TimeoutDecision -->|No| NoWaitNeeded["Rely on Playwright auto-waiting"]
SpecificPageObjectWait --> SharedObjects["Shared objects: Table, Toolbar, Pagination, Navigation"]
SharedObjects --> NoTimeout["Do not add timeout parameters in shared objects"]
PageObjectMethod --> ResponsibilityCheck["Does the method do more than one thing?"]
ResponsibilityCheck -->|Yes| SplitMethods["Split into multiple single-responsibility methods"]
ResponsibilityCheck -->|No| KeepMethodSimple["Keep method focused (e.g., clearAllFilters only clears filters)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The rule about no explicit timeouts in shared page objects could benefit from clarifying acceptable exceptions (e.g., using
expecttimeouts or handling known slow backends) to avoid confusion in edge cases. - For the composition guideline, consider explicitly distinguishing between page objects and higher-level test helpers so it’s clear where conditional guards or orchestration logic are expected to live instead of in simple action methods.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The rule about no explicit timeouts in shared page objects could benefit from clarifying acceptable exceptions (e.g., using `expect` timeouts or handling known slow backends) to avoid confusion in edge cases.
- For the composition guideline, consider explicitly distinguishing between page objects and higher-level test helpers so it’s clear where conditional guards or orchestration logic are expected to live instead of in simple action methods.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
3 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1154 +/- ##
==========================================
- Coverage 53.33% 53.30% -0.04%
==========================================
Files 270 270
Lines 5880 5878 -2
Branches 1842 1848 +6
==========================================
- Hits 3136 3133 -3
+ Misses 2443 2441 -2
- Partials 301 304 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add two E2E testing conventions to CONVENTIONS.md under the Playwright section:
Implements TC-5191
Summary by Sourcery
Document Playwright page object design conventions for shared UI components.
Documentation: