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

update test docs #1144

Merged
merged 1 commit into from
Sep 3, 2020
Merged

update test docs #1144

merged 1 commit into from
Sep 3, 2020

Conversation

sssoleileraaa
Copy link
Contributor

Remove reference to old TOTP code and update README around functional testing.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Thanks for the docs updates @creviera , went through the instructions and produced all the cassettes required for the functional tests 🎉

One minor comment inline, and a rebase on latest main due to merge rules, and these changes should be good to merge.

README.md Outdated
1. Bypass TOTP verification so that we can use the TOTP value of `123456` hard-coded in `tests/conftest.py`. You can do this by applying the following diff to the server code:

```diff
diff --git a/securedrop/models.py b/securedrop/models.py
Copy link
Contributor

Choose a reason for hiding this comment

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

this diff doesn't apply cleanly for me, with the error:

error: corrupt patch at line 13

The following diff did work for me, but looking at the previous revision, the following did appear to work for me.:

user@dev:~/src/securedrop$ git diff
diff --git a/securedrop/models.py b/securedrop/models.py
index ad991e6c3..1c954b9a6 100644
--- a/securedrop/models.py
+++ b/securedrop/models.py
@@ -658,6 +658,7 @@ class Journalist(db.Model):
 
         try:
             user = Journalist.query.filter_by(username=username).one()
+            return user
         except NoResultFound:
             raise InvalidUsernameException(
                 "invalid username '{}'".format(username))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch. i also added a gist with the patch to make it easier to download and apply.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!


1. Bypass TOTP verification so that we can use the TOTP value of `123456` hard-coded in `tests/conftest.py`. You can do this by applying the following patch to the server code:

https://gist.github.com/creviera/8793d5ec4d28f034f2c1e8320a93866a
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good idea, definitely more readable and easier to review/use, provided the author does not delete it

@emkll emkll merged commit b8c6c31 into main Sep 3, 2020
@emkll emkll deleted the update-readme branch September 3, 2020 22:58
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