Proof of Concept: Testing for docs.python.org redirects

[ewdurbin]

Description

Right now we have lots of docs redirects and they are pretty complex and difficult to scrute.

This is a proof of concept for extracting those redirects to a separate file so we can stand it up in a bare-bone nginx docker container to run automated tests against.

This should be helpful for people developing redirects and let PSF Infra more readily merge changes like #498

Requires docker and hurl to be installed locally.

chungus:psf-salt ee$ bash tests/docs/redirects/run.sh 
Error response from daemon: No such container: docs-redirects-nginx
a52b5f7a1a22f1458c3d358224c2fd6e0e3dbb25f8cc57cfc957588283d9b9a1
/Users/ee/src/psf-salt/tests/docs/redirects/tests.hurl: Success (1 request(s) in 1 ms)
--------------------------------------------------------------------------------
Executed files:    1
Executed requests: 1 (1000.0/s)
Succeeded files:   1 (100.0%)
Failed files:      0 (0.0%)
Duration:          1 ms

docs-redirects-nginx

[ewdurbin]

Interested in @AA-Turner and @JacobCoffee's thoughts here.

[JacobCoffee]

this looks good, but this just means docs contributors will need to write there tests as well as the redirect changes?

If so, rebasing #498 onto this and adding those tests would be nice!

[AA-Turner]

also cc @JulienPalard

Could we add some notes or a brief guide on how we should run this locally? I see it runs in CI, but I'd like to try and avoid pushing many commits to test something.

How extensive should the tests be? What should we be testing for? What do good hurl tests look like?

A

[AA-Turner]

Restricting branches is unhelpful for testing against CI when needed:

Suggested change

	on:
	push:
	branches: [main]
	pull_request:
	branches: [main]
	on:
	push:
	paths:
	- "salt/docs/**"
	- "test/docs/redirects/**"
	pull_request:
	paths:
	- "salt/docs/**"
	- "test/docs/redirects/**"
	workflow_dispatch:
	permissions:
	contents: read
	concurrency:
	group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
	cancel-in-progress: true

[AA-Turner]

Suggested change

	uses: actions/checkout@v3
	uses: actions/checkout@v4

[AA-Turner]

Set permissions at the workflow level

Suggested change

	permissions:
	contents: read

[ewdurbin]

Most of the specifics are up for debate. Probably the main thing to consider if the test specifications in the hurl file are acceptable.

hurl is pretty extensible for testing, but for our concern here of making sure new redirects act as expected and are well defined the status code and resulting Location are probably sufficient.

Backfilling tests for old redirects will be a chore, but probably worth doing. Big advantage would be ensuring that our existing redirects aren't broken or interfered with by new ones.

If there's value seen here, it's probably worth wrapping as much of this up into containers as possible, putting behind a make target, and documenting.

Probably more usefully, I found it much more ergonomic and faster feedback for playing with the redirect from your other outstanding PR (a few seconds per run vs running a full salt hullabaloo).

[AA-Turner]

My local state got so messy that I reset the entire thing and opened three stacked PRs (#504, #505, #506) that implement this proof-of-concept. Notably, I have generated tests for as many of the redirect rules as I could backward-induct from the regular expressions, leading to 9,406 test cases.

A

[ewdurbin]

oh snap. incredible @AA-Turner! I'll review them serially.