Skip to content

Validate HTML - #3782

Merged
jennifer-richards merged 27 commits into
ietf-tools:feat/bs5from
larseggert:validate-html
Apr 7, 2022
Merged

Validate HTML#3782
jennifer-richards merged 27 commits into
ietf-tools:feat/bs5from
larseggert:validate-html

Conversation

@larseggert

Copy link
Copy Markdown
Collaborator

With this PR, all HTML generated during the test run will be validated (with HTML-validate).

The bulk of the PR is fixes to various templates, so they actually validate.

This increases the run time of the test suite from ~20 minutes to ~24 minutes on my system. I consider this acceptable and have hence enabled this by default. There is a --no-validate-html option to turn validation off.

@larseggert
larseggert marked this pull request as ready for review April 5, 2022 13:28

@rjsparks rjsparks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really wild.
I think the spot where we end up with a csrf token outside of a form needs touching before this is merged.
It would be super nice if you could get your editor to leave file endings alone, or to customize it to leave an LF at the end of .py files.
A few other questions/comments inline

Comment thread ietf/doc/views_status_change.py
Comment thread ietf/ietfauth/views.py Outdated
Comment thread ietf/meeting/views.py Outdated
Comment thread ietf/secr/templates/base_secr.html
Comment thread ietf/templates/base/menu.html Outdated
Comment thread ietf/templates/group/review_requests.html Outdated
Comment thread ietf/templates/liaisons/detail.html
Comment thread ietf/templates/meeting/schedule_list.html

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments

Comment thread ietf/utils/test_runner.py Outdated
Comment thread ietf/utils/test_runner.py Outdated
Comment thread ietf/utils/test_runner.py

# keep the html-validate configs here, so they can be kept in sync easily
config = {}
config["frag"] = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a preferable place for settings vs settings.py?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno, good question. @rjsparks?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather have test_runner settings in test_runner than in settings.py. These aren't settings production needs, and this seems to be the right level of encapsulation for them. But instead of trying to alter the object that comes from django.config.settings perhaps there should be a module object for them instead?

Comment thread ietf/utils/test_runner.py Outdated
Comment thread ietf/templates/submit/submission_status.html
@rjsparks

rjsparks commented Apr 7, 2022

Copy link
Copy Markdown
Member

I think we can merge this now. I'll restructure the option that's being bolted onto settings later (it should go into the construction of our specialized template engine.

@jennifer-richards
jennifer-richards merged commit 3234f1a into ietf-tools:feat/bs5 Apr 7, 2022
@larseggert
larseggert deleted the validate-html branch April 7, 2022 16:45
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants