Repository navigation
Credits page automated tests [WV-2638] - #4927
amitha-m-joe wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Hi Amitha,
Thank you for raising the PR and for attaching the BrowserStack screenshot. I went through the test cases, spec, and page object. Here are a few points that can be changed for better optimization, maintainability, and to ensure the automation is validating the actual functionality described in the test case document.
1.TC-002 – Browser tab title-> the current assertion:
expect(title).toMatch(/credits|wevote/)
is too broad. It could pass even if the title is not the expected Credits page title. Please make the assertion more specific based on the expected title in the test case document, for example by verifying that it contains both "Credits & Thanks" and "WeVote", or use the exact expected title if it is fixed.
-
TC-005 – Open source software link**
Currently, the test only verifies that the
hrefexists and is truthy. A validhrefdoes not necessarily mean that the link is functional. The destination could return a 404, redirect somewhere unexpected, or otherwise fail to open.Since this test case is intended to verify that the link is clickable, please perform the click operation and verify that it opens/navigates to the expected destination.
-
TC-006 – Organization logo images
Rather than checking all
imgelements on the entire page, please scope the image elements to:CompanyWrapper-sc-r9j5wf-5 fhPjNcThis will ensure that we are validating only the organization/company logos and not unrelated images such as footer or app-store images.
Also, the logic currently around lines 42–46 can be moved into a separate reusable helper function in the page object. The spec should ideally contain only the test flow and assertion.
-
TC-007 – Organization names
Currently,
body.getText()checks the entire page. This creates a false-positive risk.For example, if "Facebook" appears in the footer, the test could pass even if the Facebook organization credit/logo is missing from the organization section.
This validation should also be scoped to:
CompanyWrapper-sc-r9j5wf-5 fhPjNcPlease move the locators and reusable logic into the page object and keep the spec focused on the test assertion.
More generally, we should avoid using
$('body').getText()when the requirement is specifically about a particular section. The same principle should be applied to other tests where applicable. -
TC-007b – Test case number and expected length
According to the test case document, TC-007b should be TC-009.
Also, rather than hard-coding
20, please declare it as a separate variable for maintainability:const minimumLength = 20;expect(logoLinks.length).toBeGreaterThanOrEqual(minimumLength);This makes it clear what the value represents for anyone maintaining the test in the future.
-
TC-010 and TC-011 – Organization logo links
These can potentially be covered together, depending on the test case document.
TC-010 should perform the actual click operation and verify that the organization link opens successfully.
Currently, TC-011 only checks:
target="_blank"This verifies that the HTML requests a new tab, but it does not verify that clicking the link actually opens a new tab. These are two different validations.
Also, please review the
orgLogoLinkslocator in the page object. If the locator already filters links usingstarts-with(@href, 'http'), then TC-010 cannot detect an invalid/non-HTTP href because those links are excluded by the locator before the test runs.The page-object locator should identify the organization links, while the test/helper should validate whether their hrefs are valid external URLs.
-
TC-014a – Volunteer role link
The current test only verifies that the
hrefhas a value and matches an HTTP URL.Since this test case is intended to verify that the volunteer role link is functional, please perform the click operation and verify that the expected page/link opens successfully.
-
C-014b and TC-015 – Missing test cases
-
TC-019 – Footer social media links
Currently, the test only verifies that the links have:
target="_blank"This verifies the HTML attribute but does not verify the actual browser behavior.
Please perform the click operation and verify that the social media link actually opens in a new tab/window as required by the test case.
-
TC-020 – Footer nonprofit disclaimer
Currently:
const bodyText = (await $('body').getText()).toLowerCase();checks the entire page and creates a false-positive risk.
Please identify the specific footer disclaimer locator and validate the required text within that specific element/section instead of checking the entire page body.
-
TC-021 – Footer open source link
Currently, the test only checks:
expect(href).toContain('github');This verifies the href value but does not verify that clicking the link actually opens GitHub.
If the test case is intended to verify the link functionality, please perform the click operation and verify that the resulting page/tab opens GitHub.
-
TC-022 – App Store badges
In addition to checking the `href` and `target`, please verify that each badge is actually displayed:
`await expect(badge).toBeDisplayed();`
-
Page Object vs. Spec separation
Wherever possible, the locators and reusable UI interaction/helper logic should remain in the page object, while the spec should primarily contain the test flow and assertions.
For example, logic such as:
- finding broken organization images
- finding invalid organization links
- identifying links that do not open in a new tab
can be implemented as reusable helper methods in the page object.
The spec can then remain simple and readable, for example:
const brokenImages = await CreditsPage.getBrokenOrganizationImages();expect(brokenImages).toEqual([]);Please keep the actual assertions in the spec rather than putting
expect()statements inside the page object. -
Avoid whole-page validation when testing a specific section
I noticed this pattern in multiple tests, particularly TC-007 and TC-020:
$('body').getText()When the requirement is related to a specific section, it is better to locate that specific section and validate its contents. This reduces false positives and makes the automation more closely aligned with the actual requirement.
-
Functional validation vs. attribute validation
In several places, the test case description says that a link should be "functional" or should "open in a new tab," but the current automation only checks attributes such as:
hrefor
target="_blank"These are useful validations, but they do not prove the actual browser behavior.
Where the requirement is functional behavior, please perform the click and verify the resulting navigation/new tab. Attribute validation can still be retained as an additional check if useful.
Thanks!
Charanya
There was a problem hiding this comment.
@amitha-m-joe and @charanya-QA would you like me to approve this pull request as-is, or wait for another iteration of this pull request to be submitted?
What github.com/wevote/WebApp/issues does this fix?
Implements automation for 14 tests regd. the Credits page tests.
Changes included this pull request?
BrowserStack results: 15/15 passing —

https://automation.browserstack.com/builds/v18rrx6fabmhgurbyc8fueac6hyoyofih8zzuj21
Notes for review:
altattributes.<a>tags wrapping an image that point off-site, rather than bya img, which returned the images themselves.beforeEachhook and there are no fixeddriver.pause()calls, per the review feedback on iPhone 5 improvements. #2637.@BVTtags since I wasn't sure these belong in the BVT suite.