Validate all generated HTML in tests - #2765
Conversation
notably, the type attribute on an a tag is supposed to hint at the target content type; did we mean to use a role tag?
|
(marking as draft because #2764 should be merged first, but other than that everything can be reviewed) |
| <a | ||
| class="btn btn-sm btn-light d-print-none" | ||
| href="{% url 'results:evaluation_text_answers_export' evaluation.id %}?view_general_results={{ view_general_results.value }}&view_contributor_results={{ view_contributor_results.value }}{% if contributor_id is not None %}&contributor_id={{ contributor_id }}{% endif %}" | ||
| type="button" |
There was a problem hiding this comment.
For this, see https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a#type
Our usage of type here doesn't make any sense, we probably meant to put a role=button? I am hesitant to put in more accessibility attributes because we don't really have a clear guideline for when to do so
| def __init__(self, *args, evaluation=None, form_id=None, **kwargs): | ||
| def __init__(self, *args, evaluation: Evaluation, form_id=None, **kwargs): |
There was a problem hiding this comment.
This was a bigger change: I don't really see why we had both evaluation and base_evaluation, I think they are always equal in our current usage? Anyways, I removed base_evaluation and passed the evaluation id in the url instead because the evaluation is needed to generate the prefix with which the form is created, so that it knows the names in request.POST to read its data from
|
|
||
|
|
||
| class ValidatingTestApp(django_webtest.DjangoTestApp): | ||
| IGNORED_ERROR_PATTERNS = ( |
There was a problem hiding this comment.
I would make a follow up issue with sub-issues for some of these; in particular the accessibility errors, the "not allowed as child of span", and the table stuff
Note that currently, we only ask for errors, but the validator also has warnings for other things :D
There was a problem hiding this comment.
I would also put relaxing the 200 HTTP status code check on that list: If we say content type is text/html, it should be valid HTML, no matter what the status code is
2 participants
Implements the idea from #1777 (comment)
We add a hook to the test app that checks all HTTP responses with HTML content against a validator. This ensures that not only our templates are linted, but also the final HTML. This catches bugs that occur only when combining multiple template files, for example duplicate html ids in a loop or cursed string quoting like #2764 (this is actually caught in the initial version of this PR until the fix is merged).
There don't seem to be many maintained HTML validators, the only serious one appears to be the Nu Html Checker (also called validator NU, VNU, and W3 Markup Validator): https://validator.nu/about.html It is associated with W3, so it's probably as good as it gets.
This pulls in a headless Java runtime (about 500MB) into our dev setup, but I think it is well worth it. To avoid having to start the JVM for every validation, we run a server as part of our process compose services with
nix run. So the whole Java business is completely hidden from the developers, they get the Java env and the running service with the normalnix runas usual.