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

Implement URL normalization in cdash_analyze_and_report.py (#577) #579

Conversation

skyreflectedinmirrors
Copy link

@skyreflectedinmirrors skyreflectedinmirrors commented May 9, 2023

This commit:

  1. Moves all URL normalization to use urllib.parse.quote from the standard library. Note: I did add a python2 fallback, which I believe should work, but I have not tested, nor do I know if that's a target at this time.
  2. Adds project name URL normalization to all URL Query creator methods (and adds appropriate tests, see: *_project_name_ampersand). Previously this was only applied in getCDashQueryTestsQueryUrl, but I felt it was best to be consistent.
  3. Selectively applies URL normalization to the project, site, build and test ids (not 100% sure the last is necessary, but doesn't hurt) to the values used in getCDashQueryTestsBrowserUrl to construct the query URLs. The quote function cannot be applied to the entire URL (which would be vastly more convenient) because, from the docs:

By default, this function is intended for quoting the path section of a URL. The optional safe parameter specifies additional ASCII characters that should not be quoted — its default value is '/'.

That is, if you apply it to the full URL it will escape things you don't want it to (e.g., the '&' in the php queries). Since we are manually constructing these, it is simple enough to apply.

  1. Adds a note about why we don't have to escape the queries coming from the input options (they are assumed to be normalized, as they're generated by CDash), and
  2. Adds a corresponding test (e.g., build name 'build++', site name with a space)

@skyreflectedinmirrors skyreflectedinmirrors changed the title Implement URL normalization in cdash_analyze_and_report.py (resolves … Implement URL normalization in cdash_analyze_and_report.py (#577) May 9, 2023
Copy link
Member

@bartlettroscoe bartlettroscoe left a comment

Choose a reason for hiding this comment

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

The changes and the tests look great! I should have now there was a standard Python library function like urlquote(). (I think I did some looking at the time but I must not have found this.)

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.

2 participants