[1545] Add E2E Tests for Browse Bills Page #1569

HuanFengYeh · 2024-06-12T01:07:59Z

Summary

Issue: #1545
This PR adds Playwright end-to-end (E2E) testing for the Browse Bills page, including:

Finding bills via text search
Sorting bills by various criteria
Filtering bills based on different attributes

My Playwright version 1.43.1 and yarn version is 1.22.19

Checklist

Playwright test covering the Browse Bills page, including:
Able to find Bills via Text Search - finished
Able to sort Bills as expected - finished
Able to filter Bills as expected- finished

Screenshots

Known issues

1.The sorting function for "sort by relevance" is not implemented as I do not have access to the relevance score needed to test this functionality.
2.Only a couple of pages were tested due to time constraints, as testing every page would take too long.
3.For the filtering function, the tests mostly focus on ensuring that the labels are showing correctly. The content itself wasn't thoroughly checked since this would require looking into the bill page in detail, which is extremely time-consuming.

Steps to test/reproduce

To run the e2e tests with the Playwright UI, run yarn test:e2e.

vercel · 2024-06-12T01:08:03Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
maple-dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 22, 2024 8:04am

…ponent.json file

Mephistic

Fantastic start!

This is the first big E2E test that will be setting an example for others going forward, so I think it's worth drilling into the details here to make sure we're setting other devs up for success going forward.

@seatuna Would love your thoughts on this. This is likely also relevant to your tests for the Browse Testimonies page, so we may be able to de-duplicate effort here.

Mephistic · 2024-06-18T16:09:13Z

components/LearnTestimonyComponents/LearnComponents.tsx

- }
-]
-
-const WriteContent = [


Love the translation work, but please make the translation changes and the new test two separate PRs.

Mephistic · 2024-06-18T16:23:13Z

tests/e2e/browse-bills.spec.ts

+ * Function to get a random word from a predefined list.
+ * @returns A random word from the list.
+ */
+ const getRandomWord = (): string => {


I feel like picking random words to search might be more complex than what we need here - it could lead to test flappiness. I would pick one of these words that consistently turns up results and just use that throughout. If/when we start making test-specific data fixtures, this would also be easier to swap out.

Our main goal here is to ensure that search works at all - not examine the specifics of how the search works (which can change with Typesense config/versions).

It could definitely make things more complicated and lead to inconsistent test results. I'll stick with a word for our tests.

Mephistic · 2024-06-18T16:51:33Z

tests/e2e/browse-bills.spec.ts

+ ) => {
+ await page.waitForFunction(initialResultCount => {
+ const currentResultCount = document.querySelector(
+ ".ResultCount__ResultContainer-sc-3931e200-0"


IIRC classes like .ResultCount__ResultContainer-sc-3931e200-0 are generated by styled components and can potentially change between builds or with trivial formatting changes - could we use a more resilient locator here (and the other places that use these classes)?

You are correct that classes like .ResultCount__ResultContainer-sc-3931e200-0 are generated by styled components and can indeed change between builds or due to trivial formatting changes, leading to unstable locators.

I am considering two alternatives and would appreciate your input on which method we should proceed with:

1.Add New Data Attribute: Adding a custom data attribute to the elements we want to target. This approach would make our locators highly stable and independent of styling changes.

2.Using Utility Classes: Utilizing existing utility classes like .flex-grow-1 and .m-1. This method offers some stability but still relies on styling changes to some extent.

For the results count text, I used page.getByText("Results").first(). If you want the text you're searching for to be more specific, you can use regex to target multiple words that will show up in that string like "showing" and "results" (this would definitely complicate it but make it more accurate) ex. page.getByText(\Results\). Just searching "Results" and selecting the first one worked out for the Testimony page.

Alternatively, adding a data-test-id is also a very good idea and would guarantee you're finding the correct element

Mephistic · 2024-06-18T16:54:37Z

tests/e2e/browse-bills.spec.ts

+
+ await page.goto(firstBillLink)
+
+ const readmorebtn = page.locator(".Summary__StyledButton-sc-791f19-3")


nit: camelCase for variable names

Thanks for point out, Corrected !

Mephistic · 2024-06-18T17:07:05Z

tests/e2e/browse-bills.spec.ts

+ * @param page - The Playwright page object.
+ * @param searchTerm - The search term to validate in the bill content.
+ */
+ const checkFirstBill = async (page: Page, searchTerm: string) => {


nit: Could we call this something like checkFirstBillResultMatchesSearchTerm to make it more clear what it's actually checking?

After chewing on this a bit, I'm not convinced we want to be mirroring the search logic in this test - given the scope of what we're trying to catch here, there are two things we could be checking here:
1.) That, should we get any search results for a term, we can follow the search result's link to the corresponding bill page.
2.) That, should we provide a specific search criteria, we find a specific bill that matches that criteria.

IMO (1) is all we need to start here. (2) is trickier (even if we set up test fixtures, we would need to avoid confounding bills and that would be a continual problem).

I don't want us to get in the business of mirroring business logic in the tests - I'm willing to rely on the Typesense tests to verify that certain search terms/facets will result in relevant results. The point of these tests should be to verify that the bones of the site work (e.g. whatever bills we get back from the search results for a term are visitable, not that the search results are relevant). Open to discussion here, but want to avoid overcomplicating this ticket or introducing fragility into the test stream.

To align with these points, I suggest we concentrate on verifying the basic functionality that the search results are visitable and ensure that search results are clickable and lead to the corresponding bill pages, confirming that navigation works as expected.

Mephistic · 2024-06-18T17:24:56Z

tests/e2e/browse-bills.spec.ts

+ * Function to click a random number of times on the "next page" button.
+ * @param page - The Playwright page object.
+ */
+ const clickNextPageRandomTimes = async (page: Page) => {


Mentioned above, but I feel like we should avoid randomness in tests where possible - if we can navigate forward once and back once, I'd be satisified - we get diminishing returns vis-a-vis verifying correctness with each additional page we check.

I agree that we should aim for consistent and reliable tests. I will avoid randomness and focusing on a single navigation forward and back.

Mephistic · 2024-06-18T17:28:58Z

tests/e2e/browse-bills.spec.ts

+ await performSearch(page, searchTerm)
+
+ const noResultsMessage = await page.textContent(
+ ".NoResults__Container-sc-bf043801-0 .text-center"


OOC, are these the locators that Playwright is generating via VS Code / whatever IDE integration you're using (e.g. https://playwright.dev/docs/getting-started-vscode#picking-a-locator)? More curious for the implications for other tests going forward than for this PR, but still curious. We should feel free to add data-testids where necessary if it supports the testing effort.

I retrieved the locators by inspecting the HTML code using Google Chrome's Developer Tools. The locators are based on the frontend HTML and CSS classes that are rendered by the browser. These can be generated by frontend frameworks like React and styled with CSS or CSS-in-JS libraries.

It will definitely make the test more reliable by adding data-testid attributes to key elements in our application to create more stable locators. This will help us ensure that our tests remain robust against UI changes.

Mephistic · 2024-06-18T17:29:37Z

tests/e2e/browse-bills.spec.ts

+
+// Array of sorting test configurations
+const sortingTests: SortingTest[] = [
+ // {


nit: If we're intentionally leaving this out (which I believe we are IIRC), we should leave a comment that this is intentionally excluded.

I'll add a comment to the code!

Mephistic · 2024-06-18T17:41:13Z

tests/e2e/browse-bills.spec.ts

+ * @returns The extracted value as a number.
+ */
+const getAttributeValue = async (
+ item: ElementHandle<Element>,


It looks like Playwright wants us to move away from ElementHandles to using locators (https://playwright.dev/docs/api/class-elementhandle#element-handle-query-selector-all) - could we update this logic to handle that change?

I wasn't aware that Playwright is moving away from ElementHandles in favor of using locators. It's good to know about this update!

I agree that we should switch our logic to use locators as recommended. I'll start updating the relevant parts of our codebase to handle this change.

Mephistic · 2024-06-18T17:53:11Z

tests/e2e/browse-bills.spec.ts

+ * @param filterCategory - The selector of the filter category.
+ * @returns The selector for a randomly chosen filter item.
+ */
+ const getRandomFilterItemSelector = async (


Let's avoid randomness here as well for consistency - I'd just pick an arbitrary filter option for each list and use that.

seatuna · 2024-06-21T19:14:11Z

This is a great start! Some general comments I have:

I'd avoid using CSS classes for the locators, especially since there are generated CSS glasses that may not always be the same every time. I find that the getBy... locators work really well because it forces you to look at the page from a user's perspective, and they also have built in retries, so it may not be necessary to use manual waits. https://playwright.dev/docs/locators
I only just found this feature recently, but Playwright's Page Object Model will be very useful for frequently used elements or actions: https://playwright.dev/docs/pom
Still in progress, but if you want to see my WIP files:
https://github.com/seatuna/maple/blob/1546_browse-testimonies-e2e-tests/tests/e2e/testimony.spec.ts
https://github.com/seatuna/maple/blob/1546_browse-testimonies-e2e-tests/tests/e2e/page_objects/testimony.ts (page object model for the Testimonies page).

huanfeng added 12 commits May 31, 2024 19:00

Update playwright test for find Bills via Text Search

f7f1f6c

Begin implementation of test for sorting bills

3b68679

Implement clicking sort options for sort bills test

9db1a04

Implement test for sorting function excpet 'sort by relevance'

0aaf7bb

Begin implementation of test for filtering bills

29c38e8

Update playwright test for feature1 and featrue 2 remaining

ba9c732

Update playwright test for feature 1 completed and feature 3 remaining

467f407

Update the test for filtering bills

9d1b2f6

Update the test for sorting bills and filtering bills(remaining)

91f797b

Contiune working on filter and sorting test

d27cde8

Finished Playwright test for filtering functionality for Bills

91f900e

Finsihed the searching sorting filtering test for bills page

6190399

HuanFengYeh requested review from sashamaryl, mertbagt, nesanders, mvictor55, timblais, alexjball and Mephistic as code owners June 12, 2024 01:08

vercel bot deployed to Preview – maple-dev June 12, 2024 01:13 View deployment

Extract user-visible context from learnComponents and create learnCom…

f3ce69b

…ponent.json file

vercel bot deployed to Preview – maple-dev June 18, 2024 08:59 View deployment

Finished isuue codeforboston#946

453052a

vercel bot deployed to Preview – maple-dev June 18, 2024 12:56 View deployment

Mephistic requested changes Jun 18, 2024

View reviewed changes

Seperating the translation changes

4faa88b

vercel bot deployed to Preview – maple-dev June 22, 2024 06:30 View deployment

Modified getRandomWord to getSearchWord

ac2cb7d

vercel bot deployed to Preview – maple-dev June 22, 2024 07:17 View deployment

Corrected no camelCaes variable readmorebtn

75e8627

vercel bot deployed to Preview – maple-dev June 22, 2024 08:04 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1545] Add E2E Tests for Browse Bills Page #1569

[1545] Add E2E Tests for Browse Bills Page #1569

HuanFengYeh commented Jun 12, 2024 •

edited

Loading

vercel bot commented Jun 12, 2024 •

edited

Loading

Mephistic left a comment

Mephistic Jun 18, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 22, 2024 •

edited

Loading

Mephistic Jun 18, 2024

HuanFengYeh Jun 22, 2024 •

edited

Loading

seatuna Jun 24, 2024

seatuna Jun 24, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 22, 2024

Mephistic Jun 18, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 22, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 23, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 23, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 23, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 25, 2024

Mephistic Jun 18, 2024

HuanFengYeh Jun 25, 2024

seatuna commented Jun 21, 2024


		await page.goto(firstBillLink)

		const readmorebtn = page.locator(".Summary__StyledButton-sc-791f19-3")

[1545] Add E2E Tests for Browse Bills Page #1569

Are you sure you want to change the base?

[1545] Add E2E Tests for Browse Bills Page #1569

Conversation

HuanFengYeh commented Jun 12, 2024 • edited Loading

Summary

Checklist

Screenshots

Known issues

Steps to test/reproduce

vercel bot commented Jun 12, 2024 • edited Loading

Mephistic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuanFengYeh Jun 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuanFengYeh Jun 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seatuna commented Jun 21, 2024

HuanFengYeh commented Jun 12, 2024 •

edited

Loading

vercel bot commented Jun 12, 2024 •

edited

Loading

HuanFengYeh Jun 22, 2024 •

edited

Loading

HuanFengYeh Jun 22, 2024 •

edited

Loading