TestURLs coverage for (most of) console #2108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 12, 2023 16:06
If applications have been submitted during a particular year, but no applications from that year have (yet) been accepted or rejected, then it is impossible to compute the percentage of accepted applications (since pending and withdrawn applications aren't counted.) (For example, the demo data currently includes a pending application from year 2022, but no accepted or rejected applications.) Set this value to None, as is done with other metrics that cannot be computed.
A few URLs in this app cannot currently be tested due to a lack of demo data; in particular, testing the project submission pipeline would require creating a demo project for each submission stage. Many URLs in this app use a parameter called, uninformatively, "pk". In most such cases, setting "pk" to 1 will work; but it'd be wiser to use a descriptive name for the parameter. It'd also be wiser not to use integer IDs in URLs to begin with, and it would be wiser not to assume that integer IDs are also primary keys. Also note that user_access_logs_detail misleadingly names its parameter "pid" (it's a user ID, not any kind of project ID.)
|
thanks! |
tompollard
added a commit
that referenced
this pull request
May 14, 2024
This adds test coverage for most of the URLs in the `project` app. See the previous pull requests #1695, #1922, #2108 for background. With this change, the bulk of site functionality should now be included in the TestURLs framework. A few small apps are left to add: - `oauth` (should be easy) - `sso` (should be easy but I'm not sure this is enabled in test configs) - `training` (awaiting pull #1951) And a number of URLs across the site are skipped due to a lack of fixture data: - `console` - `edit_submission` - `copyedit_submission` - `awaiting_authors` - `publish_submission` - `event_agreement_detail` - `event_agreement_delete` - `event_agreement_new_version` - `project` - `serve_document` - `published_project_request_access` - `search` - `redirect_challenge_project`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds test coverage for most of the URLs in the console app.
(See pull #1695 for an overview of how URL testing works.)
A few URLs can't be tested and some of these test cases aren't really meaningful, but overall this adds a fair amount of coverage. I want to do this because the console templates could use a lot of improvement.
Making an exhaustive list also helps to highlight some of the poor logic that I wasn't aware of. Look at these lovely warnings, pointing out that the code is doing something it shouldn't:
(Oh, and I fixed a bug in the credentialing stats.)