Update screenshot suite URLs#6261
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request optimizes the configuration for Percy screenshot tests by significantly reducing the number of tracked URLs. The changes focus on improving the quality and diversity of the test suite, ensuring better coverage of core site features and representative data queries while maintaining a streamlined and efficient testing process. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Please help me ensure that critical parameters and paths are represented, thanks! |
There was a problem hiding this comment.
Code Review
This pull request reorganizes and prunes the urls.json file used for Percy snapshots, adding new landing pages for the home, about, and FAQ sections while standardizing visualization tool redirects. A significant number of specific explore query URLs were removed to streamline the snapshot process. Feedback was provided to standardize the /explore URL paths by removing trailing slashes before fragment identifiers to maintain consistency and prevent unnecessary redirects.
| "path": "/explore/#q=Obesity+vs+Unemployment+in+the+US+counties&mb=3&test=screenshot", | ||
| "name": "Explore: Obesity vs. Unemployment in US Counties" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=Businesses+by+industry+in+New+York+State&mb=3&test=screenshot", | ||
| "name": "Explore: Businesses by Industry in New York State" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=Health+equity+in+Santa+Clara+County&mb=3&test=screenshot", | ||
| "name": "Explore: Health Equity in Santa Clara County" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=Incarceration+rates+by+race+in+the+US&mb=3&test=screenshot", | ||
| "name": "Explore: Incarceration Rates by Race in the US" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=health+behaviors+in+India+vs+Norway&mb=3&test=screenshot", | ||
| "name": "Explore: Health Behaviors in India vs. Norway" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=High+blood+pressure+vs+Asthma+in+Alabama&mb=3&test=screenshot", | ||
| "name": "Explore: High Blood Pressure vs. Asthma in Alabama" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=School+Enrollment+in+California&mb=3&test=screenshot", | ||
| "name": "Explore: School Enrollment in California" | ||
| }, | ||
| { | ||
| "path": "/explore/#q=Student+demographics+among+schools+in+Sunnyvale&mb=3&test=screenshot", | ||
| "name": "Explore: Student Demographics in Sunnyvale Schools" | ||
| }, | ||
| { |
There was a problem hiding this comment.
The URL paths for these explore queries use a trailing slash before the fragment identifier (/explore/#), while other explore URLs in this file (e.g., lines 103, 107, 123) do not. Standardizing on /explore# is preferred to maintain consistency and avoid potential unnecessary redirects during the snapshot process.
{
"path": "/explore#q=Obesity+vs+Unemployment+in+the+US+counties&mb=3&test=screenshot",
"name": "Explore: Obesity vs. Unemployment in US Counties"
},
{
"path": "/explore#q=health+behaviors+in+India+vs+Norway&mb=3&test=screenshot",
"name": "Explore: Health Behaviors in India vs. Norway"
},
{
"path": "/explore#q=Student+demographics+among+schools+in+Sunnyvale&mb=3&test=screenshot",
"name": "Explore: Student Demographics in Sunnyvale Schools"
},
gmechali
left a comment
There was a problem hiding this comment.
Thanks for cleaning this up!
juliawu
left a comment
There was a problem hiding this comment.
Thank you for the cleanup! Just one comment.
nick-nlb
left a comment
There was a problem hiding this comment.
Likewise, thank you for this cleanup!
I went through the updated list and commented on a couple to put back in. Otherwise looks good.
|
thanks for the reviews! updated based on comments - @juliawu could you re-approve? |
juliawu
left a comment
There was a problem hiding this comment.
Thanks for updating these!
I missed the starting slash in one URL from the last PR #6261 :( ptal..
This PR curates the list of URLs used for Percy screenshot tests in urls.json. The previous list was excessively long (over 1000 lines) and repetitive, while still lacking coverage for several key features and parameters.
Key Changes
Verification
Verified that the updated file parses correctly as valid JSON.