-
Notifications
You must be signed in to change notification settings - Fork 685
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
Bring new fixtures to Journalist interface layout tests #6504
Bring new fixtures to Journalist interface layout tests #6504
Conversation
5b23420
to
493d4da
Compare
493d4da
to
c1b4fc3
Compare
|
||
|
||
@pytest.mark.parametrize("locale", list_locales()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below I merged some of the "screenshot-taking" tests, so that the test suite would run faster.
|
||
def test_index_no_documents_admin(self): | ||
self._admin_logs_in() | ||
self._screenshot("journalist-admin_index_no_documents.png") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This screenshot is still referenced in the documentation.
self._source_submits_a_message() | ||
self._source_logs_out() | ||
self._journalist_logs_in() | ||
self._screenshot("journalist-index_javascript.png") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This screenshot is still referenced in the documentation
|
||
def test_index_entered_text(self): | ||
self._input_text_in_login_form("jane_doe", "my password is long", "117264") | ||
self._screenshot("journalist-index_with_text.png") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This screenshot is still referenced in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change overall, 3 of the tests that were removed generate screenshots that are still used in the docs, so they may need to be re-added (or the shots grabbed elsewhere in the test flow)
@zenmonkeykstop Good catch - I didn't realize that this was how the screenshots were used. I've re-added the missing ones, and left the changes as a separate commit so that it's easier to review. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the update.
Status
Ready
Description of Changes
This PR continue the work from #6503, by bringing the new test code and fixtures to some of the layout tests for the journalist app.
More specifically, this PR: