WordPress / openverse

Openverse is a search engine for openly-licensed media. This monorepo includes all application code.
https://openverse.org
MIT License
254 stars 204 forks source link

Combine frontend testing documentation pages #4514

Closed sarayourfriend closed 4 months ago

sarayourfriend commented 5 months ago

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 https://github.com/WordPress/openverse/pull/4499#issuecomment-2176026358

Additional context

Make sure to add redirects.

madewithkode commented 5 months ago

Hi @sarayourfriend quick questions:

sarayourfriend commented 5 months ago

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 commented 5 months ago

Really helpful insights as always, thank you!