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

Regression test for valid HTML being generated in the error path. #182

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

albu-diku
Copy link

@albu-diku albu-diku commented Jan 25, 2025

This PR is a test only addition that adds a regression test for the fix that landed as c545adc, the original PR being #178. The test was written at the same time as the original fix but has additional dependencies (#98). With the pre-requisite now in-tree we can now land this and address the follow-up item.

@albu-diku albu-diku changed the base branch from edge to test/migwsgi January 28, 2025 09:43
@albu-diku albu-diku force-pushed the test/migwsgi branch 3 times, most recently from af91424 to 6a6add4 Compare January 29, 2025 11:15
@albu-diku albu-diku force-pushed the test/migwsgi branch 3 times, most recently from ff85267 to e48b12e Compare February 5, 2025 19:27
Base automatically changed from test/migwsgi to next February 6, 2025 09:44
@jonasbardino
Copy link
Contributor

jonasbardino commented Feb 8, 2025

I wonder what happened here with all those commits and apparently unrelated files changed?
Perhaps the change from edge to next branch actually ended up introducing the full changeset between the two here. I saw similar for a PR when I inadvertently made it against the other branch than I had started from.

@albu-diku albu-diku force-pushed the test/invalid-html-on-error branch from a8582b9 to b0a3f93 Compare February 11, 2025 09:05
@alexjeffburke
Copy link

I’m not sure exactly what happened here either. Unfortunately we also landed something a bit different to what this was sat atop so it had also bitrotted - I’ve rewritten the validating assertion along the lines of the title checker and pushed it back up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants