Skip to content
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

Combine frontend testing documentation pages #4514

Closed
sarayourfriend opened this issue Jun 18, 2024 · 3 comments · Fixed by #4546
Closed

Combine frontend testing documentation pages #4514

sarayourfriend opened this issue Jun 18, 2024 · 3 comments · Fixed by #4546
Assignees
Labels
📄 aspect: text Concerns the textual material in the repository ✨ goal: improvement Improvement to an existing user-facing feature help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: documentation Related to Sphinx documentation

Comments

@sarayourfriend
Copy link
Collaborator

Current Situation

The frontend testing documentation is currently split between two pages, in two separate sections:

https://docs.openverse.org/frontend/guides/test.html
https://docs.openverse.org/frontend/reference/playwright_tests.html

The latter is linked by the playwright bot comment, but only the former includes instructions for actually updating the playwright test snapshots.

Suggested Improvement

We should combine these documents into a single frontend testing documentation page. Snapshots are now used on both playwright and jest tests, so it's important that these instructions are visible and easy to find for all cases of frontend testing. The division of the two is confusing anyway.

  1. Move the testing guide into a single testing reference page
  2. Update the playwright bot comment to link to the right section of the new page
  3. Add redirects in the documentation configuration so old links continue to work

Benefit

Prevent confusion like #4499 (comment)

Additional context

Make sure to add redirects.

@sarayourfriend sarayourfriend added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Jun 18, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Jun 18, 2024
@sarayourfriend sarayourfriend added help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 📄 aspect: text Concerns the textual material in the repository 🧱 stack: documentation Related to Sphinx documentation ✨ goal: improvement Improvement to an existing user-facing feature and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Jun 19, 2024
@madewithkode
Copy link
Collaborator

Hi @sarayourfriend quick questions:

  • Regarding adding redirects in the documentation configuration so old links continue to work, what would be the best way to do this? Off the top of my head, this would basically be manually searching through the code base and replacing existing references with the new links. I'd like to assume there's a better way.

  • About Updating the playwright bot comment to link to the right section of the new page, can you also help with pointing me to the part of the codebase that houses the bot logic and also how to test changes to that locally(maybe).

@sarayourfriend
Copy link
Collaborator Author

Sure thing. Redirects are handled in this block of the documentation configuration: https://github.com/WordPress/openverse/blob/main/documentation/conf.py#L90-L106. It comes from this Sphinx plugin (docs link).

For actual links in the doc, you will need to update them because the Sphinx build will fail on unresolvable references within the documentation site. I don't see any that aren't part of these specific two pages anyway, so I don't think you'll need to update references within the documentation site (or other Openverse docs). Adding the redirects is sufficient for handling other references (like old bot comments).

The bot comment comes from this job in ci_cd.yml: https://github.com/WordPress/openverse/blob/main/.github/workflows/ci_cd.yml#L762-L822

Here's the specific line with the link to edit for the comment: https://github.com/WordPress/openverse/blob/main/.github/workflows/ci_cd.yml#L802

To test the bot comment (which I don't think is necessary in this case, because it's just swapping the link out), add a commit that forces the playwright tests to fail and check the comment left by the bot. Those workflows run on the branch (not on main) so you can iterate as much as you need by causing the bot to leave a comment for any given commit and see what the output is. Then remove whatever you added to force the failure before merging, of course.

@madewithkode
Copy link
Collaborator

Really helpful insights as always, thank you!

@madewithkode madewithkode self-assigned this Jun 20, 2024
@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Jun 20, 2024
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Jun 24, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 aspect: text Concerns the textual material in the repository ✨ goal: improvement Improvement to an existing user-facing feature help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: documentation Related to Sphinx documentation
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants