From efa5d5ef2cac7510ae1e2b743124634939643dce Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 11:37:06 -0800 Subject: [PATCH 01/11] Adds ci test runner --- .github/workflows/django-tests.yml | 68 ++++++++++++++++++++++++++++++ bw-dev | 3 +- 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/django-tests.yml diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml new file mode 100644 index 000000000..3ce368ecd --- /dev/null +++ b/.github/workflows/django-tests.yml @@ -0,0 +1,68 @@ +name: Run Python Tests +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + build: + + runs-on: ubuntu-20.04 + strategy: + max-parallel: 4 + matrix: + db: [postgres] + python-version: [3.9] + include: + - db: postgres + db_port: 5432 + + services: + postgres: + image: postgres:10 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: hunter2 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Run Tests + env: + DB: ${{ matrix.db }} + DB_HOST: 127.0.0.1 + DB_PORT: ${{ matrix.db_port }} + DB_PASSWORD: hunter2 + SECRET_KEY: beepbeep + DEBUG: true + DOMAIN: your.domain.here + OL_URL: https://openlibrary.org + BOOKWYRM_DATABASE_BACKEND: postgres + MEDIA_ROOT: images/ + POSTGRES_PASSWORD: hunter2 + POSTGRES_USER: postgres + POSTGRES_DB: github_actions + POSTGRES_HOST: 127.0.0.1 + CELERY_BROKER: "" + CELERY_RESULT_BACKEND: "" + EMAIL_HOST: "smtp.mailgun.org" + EMAIL_PORT: 587 + EMAIL_HOST_USER: "" + EMAIL_HOST_PASSWORD: "" + EMAIL_USE_TLS: true + run: | + python manage.py test diff --git a/bw-dev b/bw-dev index 6d74924ff..53c8e52d9 100755 --- a/bw-dev +++ b/bw-dev @@ -61,7 +61,8 @@ case "$1" in ;; migrate) execweb python manage.py rename_app fedireads bookwyrm - execweb python manage.py "$@" + shift 1 + execweb python manage.py migrate "$@" ;; bash) execweb bash From 257a29dcfdc2576f6d19d623f29cf0d7aafdee72 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 11:53:30 -0800 Subject: [PATCH 02/11] Comment out failing tests Obviously this is not a SOLUTION, it's an intermediary step in resolving the redis dependency issues. this PR isn't mergable until the tests are restored. --- bookwyrm/tests/incoming/test_follow.py | 46 +++---- bookwyrm/tests/outgoing/test_follow.py | 102 +++++++-------- bookwyrm/tests/outgoing/test_shelving.py | 72 +++++------ bookwyrm/tests/test_signing.py | 156 +++++++++++------------ 4 files changed, 188 insertions(+), 188 deletions(-) diff --git a/bookwyrm/tests/incoming/test_follow.py b/bookwyrm/tests/incoming/test_follow.py index 51ab3c43a..377c70205 100644 --- a/bookwyrm/tests/incoming/test_follow.py +++ b/bookwyrm/tests/incoming/test_follow.py @@ -18,29 +18,29 @@ class IncomingFollow(TestCase): self.local_user.save() - def test_handle_follow(self): - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/users/rat", - "object": "http://local.com/user/mouse" - } - - incoming.handle_follow(activity) - - # notification created - notification = models.Notification.objects.get() - self.assertEqual(notification.user, self.local_user) - self.assertEqual(notification.notification_type, 'FOLLOW') - - # the request should have been deleted - requests = models.UserFollowRequest.objects.all() - self.assertEqual(list(requests), []) - - # the follow relationship should exist - follow = models.UserFollows.objects.get(user_object=self.local_user) - self.assertEqual(follow.user_subject, self.remote_user) +# def test_handle_follow(self): +# activity = { +# "@context": "https://www.w3.org/ns/activitystreams", +# "id": "https://example.com/users/rat/follows/123", +# "type": "Follow", +# "actor": "https://example.com/users/rat", +# "object": "http://local.com/user/mouse" +# } +# +# incoming.handle_follow(activity) +# +# # notification created +# notification = models.Notification.objects.get() +# self.assertEqual(notification.user, self.local_user) +# self.assertEqual(notification.notification_type, 'FOLLOW') +# +# # the request should have been deleted +# requests = models.UserFollowRequest.objects.all() +# self.assertEqual(list(requests), []) +# +# # the follow relationship should exist +# follow = models.UserFollows.objects.get(user_object=self.local_user) +# self.assertEqual(follow.user_subject, self.remote_user) def test_handle_follow_manually_approved(self): diff --git a/bookwyrm/tests/outgoing/test_follow.py b/bookwyrm/tests/outgoing/test_follow.py index 82a476f62..a1ea3f690 100644 --- a/bookwyrm/tests/outgoing/test_follow.py +++ b/bookwyrm/tests/outgoing/test_follow.py @@ -19,54 +19,54 @@ class Following(TestCase): ) - def test_handle_follow(self): - self.assertEqual(models.UserFollowRequest.objects.count(), 0) - - outgoing.handle_follow(self.local_user, self.remote_user) - rel = models.UserFollowRequest.objects.get() - - self.assertEqual(rel.user_subject, self.local_user) - self.assertEqual(rel.user_object, self.remote_user) - self.assertEqual(rel.status, 'follow_request') - - - def test_handle_unfollow(self): - self.remote_user.followers.add(self.local_user) - self.assertEqual(self.remote_user.followers.count(), 1) - outgoing.handle_unfollow(self.local_user, self.remote_user) - - self.assertEqual(self.remote_user.followers.count(), 0) - - - def test_handle_accept(self): - rel = models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - rel_id = rel.id - - outgoing.handle_accept(rel) - # request should be deleted - self.assertEqual( - models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 - ) - # follow relationship should exist - self.assertEqual(self.remote_user.followers.first(), self.local_user) - - - def test_handle_reject(self): - rel = models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - rel_id = rel.id - - outgoing.handle_reject(rel) - # request should be deleted - self.assertEqual( - models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 - ) - # follow relationship should not exist - self.assertEqual( - models.UserFollows.objects.filter(id=rel_id).count(), 0 - ) +# def test_handle_follow(self): +# self.assertEqual(models.UserFollowRequest.objects.count(), 0) +# +# outgoing.handle_follow(self.local_user, self.remote_user) +# rel = models.UserFollowRequest.objects.get() +# +# self.assertEqual(rel.user_subject, self.local_user) +# self.assertEqual(rel.user_object, self.remote_user) +# self.assertEqual(rel.status, 'follow_request') +# +# +# def test_handle_unfollow(self): +# self.remote_user.followers.add(self.local_user) +# self.assertEqual(self.remote_user.followers.count(), 1) +# outgoing.handle_unfollow(self.local_user, self.remote_user) +# +# self.assertEqual(self.remote_user.followers.count(), 0) +# +# +# def test_handle_accept(self): +# rel = models.UserFollowRequest.objects.create( +# user_subject=self.local_user, +# user_object=self.remote_user +# ) +# rel_id = rel.id +# +# outgoing.handle_accept(rel) +# # request should be deleted +# self.assertEqual( +# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 +# ) +# # follow relationship should exist +# self.assertEqual(self.remote_user.followers.first(), self.local_user) +# +# +# def test_handle_reject(self): +# rel = models.UserFollowRequest.objects.create( +# user_subject=self.local_user, +# user_object=self.remote_user +# ) +# rel_id = rel.id +# +# outgoing.handle_reject(rel) +# # request should be deleted +# self.assertEqual( +# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 +# ) +# # follow relationship should not exist +# self.assertEqual( +# models.UserFollows.objects.filter(id=rel_id).count(), 0 +# ) diff --git a/bookwyrm/tests/outgoing/test_shelving.py b/bookwyrm/tests/outgoing/test_shelving.py index 0b85b671b..c26bec84f 100644 --- a/bookwyrm/tests/outgoing/test_shelving.py +++ b/bookwyrm/tests/outgoing/test_shelving.py @@ -25,39 +25,39 @@ class Shelving(TestCase): ) - def test_handle_shelve(self): - outgoing.handle_shelve(self.user, self.book, self.shelf) - # make sure the book is on the shelf - self.assertEqual(self.shelf.books.get(), self.book) - - - def test_handle_shelve_to_read(self): - shelf = models.Shelf.objects.get(identifier='to-read') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_shelve_reading(self): - shelf = models.Shelf.objects.get(identifier='reading') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_shelve_read(self): - shelf = models.Shelf.objects.get(identifier='read') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_unshelve(self): - self.shelf.books.add(self.book) - self.shelf.save() - self.assertEqual(self.shelf.books.count(), 1) - outgoing.handle_unshelve(self.user, self.book, self.shelf) - self.assertEqual(self.shelf.books.count(), 0) +# def test_handle_shelve(self): +# outgoing.handle_shelve(self.user, self.book, self.shelf) +# # make sure the book is on the shelf +# self.assertEqual(self.shelf.books.get(), self.book) +# +# +# def test_handle_shelve_to_read(self): +# shelf = models.Shelf.objects.get(identifier='to-read') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_shelve_reading(self): +# shelf = models.Shelf.objects.get(identifier='reading') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_shelve_read(self): +# shelf = models.Shelf.objects.get(identifier='read') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_unshelve(self): +# self.shelf.books.add(self.book) +# self.shelf.save() +# self.assertEqual(self.shelf.books.count(), 1) +# outgoing.handle_unshelve(self.user, self.book, self.shelf) +# self.assertEqual(self.shelf.books.count(), 0) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 5170b4167..bdd82585a 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -70,9 +70,9 @@ class Signature(TestCase): signer or sender, self.rat.inbox, now, digest) return self.send(signature, now, send_data or data, digest) - def test_correct_signature(self): - response = self.send_test_request(sender=self.mouse) - self.assertEqual(response.status_code, 200) +# def test_correct_signature(self): +# response = self.send_test_request(sender=self.mouse) +# self.assertEqual(response.status_code, 200) def test_wrong_signature(self): ''' Messages must be signed by the right actor. @@ -80,82 +80,82 @@ class Signature(TestCase): response = self.send_test_request(sender=self.mouse, signer=self.cat) self.assertEqual(response.status_code, 401) - @responses.activate - def test_remote_signer(self): - ''' signtures for remote users ''' - datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') - data = json.loads(datafile.read_bytes()) - data['id'] = self.fake_remote.remote_id - data['publicKey']['publicKeyPem'] = self.fake_remote.public_key - del data['icon'] # Avoid having to return an avatar. - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - responses.add( - responses.GET, - 'https://localhost/.well-known/nodeinfo', - status=404) - responses.add( - responses.GET, - 'https://example.com/user/mouse/outbox?page=true', - json={'orderedItems': []}, - status=200 - ) +# @responses.activate +# def test_remote_signer(self): +# ''' signtures for remote users ''' +# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') +# data = json.loads(datafile.read_bytes()) +# data['id'] = self.fake_remote.remote_id +# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key +# del data['icon'] # Avoid having to return an avatar. +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# responses.add( +# responses.GET, +# 'https://localhost/.well-known/nodeinfo', +# status=404) +# responses.add( +# responses.GET, +# 'https://example.com/user/mouse/outbox?page=true', +# json={'orderedItems': []}, +# status=200 +# ) +# +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - @responses.activate - def test_key_needs_refresh(self): - datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') - data = json.loads(datafile.read_bytes()) - data['id'] = self.fake_remote.remote_id - data['publicKey']['publicKeyPem'] = self.fake_remote.public_key - del data['icon'] # Avoid having to return an avatar. - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - responses.add( - responses.GET, - 'https://localhost/.well-known/nodeinfo', - status=404) - responses.add( - responses.GET, - 'https://example.com/user/mouse/outbox?page=true', - json={'orderedItems': []}, - status=200 - ) - - # Second and subsequent fetches get a different key: - new_private_key, new_public_key = create_key_pair() - new_sender = Sender( - self.fake_remote.remote_id, new_private_key, new_public_key) - data['publicKey']['publicKeyPem'] = new_public_key - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - - # Key correct: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - # Old key is cached, so still works: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - # Try with new key: - response = self.send_test_request(sender=new_sender) - self.assertEqual(response.status_code, 200) - - # Now the old key will fail: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 401) +# @responses.activate +# def test_key_needs_refresh(self): +# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') +# data = json.loads(datafile.read_bytes()) +# data['id'] = self.fake_remote.remote_id +# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key +# del data['icon'] # Avoid having to return an avatar. +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# responses.add( +# responses.GET, +# 'https://localhost/.well-known/nodeinfo', +# status=404) +# responses.add( +# responses.GET, +# 'https://example.com/user/mouse/outbox?page=true', +# json={'orderedItems': []}, +# status=200 +# ) +# +# # Second and subsequent fetches get a different key: +# new_private_key, new_public_key = create_key_pair() +# new_sender = Sender( +# self.fake_remote.remote_id, new_private_key, new_public_key) +# data['publicKey']['publicKeyPem'] = new_public_key +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# +# # Key correct: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) +# +# # Old key is cached, so still works: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) +# +# # Try with new key: +# response = self.send_test_request(sender=new_sender) +# self.assertEqual(response.status_code, 200) +# +# # Now the old key will fail: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 401) @responses.activate From 44a0ef3b0b6775eadb0e2632fc7e37c7271adc5c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 12:25:01 -0800 Subject: [PATCH 03/11] Fixes nondeterministic order of query causing test failure --- bookwyrm/tests/models/test_user_model.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 203c558d7..611e1fd68 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -27,7 +27,9 @@ class User(TestCase): shelves = models.Shelf.objects.filter(user=self.user).all() self.assertEqual(len(shelves), 3) names = [s.name for s in shelves] - self.assertEqual(names, ['To Read', 'Currently Reading', 'Read']) + self.assertTrue('To Read' in names) + self.assertTrue('Currently Reading' in names) + self.assertTrue('Read' in names) ids = [s.identifier for s in shelves] self.assertEqual(ids, ['to-read', 'reading', 'read']) From 0c01af4042f13913e7c3b4ac8ba79fbc33f5af4a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 12:42:01 -0800 Subject: [PATCH 04/11] Another nondeterministic list order problem --- bookwyrm/tests/models/test_user_model.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 611e1fd68..e82f91be1 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -31,7 +31,9 @@ class User(TestCase): self.assertTrue('Currently Reading' in names) self.assertTrue('Read' in names) ids = [s.identifier for s in shelves] - self.assertEqual(ids, ['to-read', 'reading', 'read']) + self.assertTrue('to-read' in ids) + self.assertTrue('reading' in ids) + self.assertTrue('read' in ids) def test_activitypub_serialize(self): From 48ab993861ddb400338e2c46f8d14ecfc719ddbc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:02:26 -0800 Subject: [PATCH 05/11] Mocks celery task for follow request --- bookwyrm/tests/test_signing.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index bdd82585a..35c68cf5c 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -2,6 +2,7 @@ import time from collections import namedtuple from urllib.parse import urlsplit import pathlib +from unittest.mock import patch import json import responses @@ -63,16 +64,18 @@ class Signature(TestCase): send_data=None, digest=None, date=None): + ''' sends a follow request to the "rat" user ''' now = date or http_date() data = json.dumps(get_follow_data(sender, self.rat)) digest = digest or make_digest(data) signature = make_signature( signer or sender, self.rat.inbox, now, digest) - return self.send(signature, now, send_data or data, digest) + with patch('bookwyrm.incoming.handle_follow.delay') as _: + return self.send(signature, now, send_data or data, digest) -# def test_correct_signature(self): -# response = self.send_test_request(sender=self.mouse) -# self.assertEqual(response.status_code, 200) + def test_correct_signature(self): + response = self.send_test_request(sender=self.mouse) + self.assertEqual(response.status_code, 200) def test_wrong_signature(self): ''' Messages must be signed by the right actor. From 73279d65d7af633c8a3674a1006123e06994cfdf Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:08:01 -0800 Subject: [PATCH 06/11] Fix test remote signer and comment out failing tests --- bookwyrm/tests/test_signing.py | 95 +++++++++++++++++----------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 35c68cf5c..d57a34392 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -83,32 +83,33 @@ class Signature(TestCase): response = self.send_test_request(sender=self.mouse, signer=self.cat) self.assertEqual(response.status_code, 401) -# @responses.activate -# def test_remote_signer(self): -# ''' signtures for remote users ''' -# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') -# data = json.loads(datafile.read_bytes()) -# data['id'] = self.fake_remote.remote_id -# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key -# del data['icon'] # Avoid having to return an avatar. -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# responses.add( -# responses.GET, -# 'https://localhost/.well-known/nodeinfo', -# status=404) -# responses.add( -# responses.GET, -# 'https://example.com/user/mouse/outbox?page=true', -# json={'orderedItems': []}, -# status=200 -# ) -# -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) + @responses.activate + def test_remote_signer(self): + ''' signtures for remote users ''' + datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') + data = json.loads(datafile.read_bytes()) + data['id'] = self.fake_remote.remote_id + data['publicKey']['publicKeyPem'] = self.fake_remote.public_key + del data['icon'] # Avoid having to return an avatar. + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + responses.add( + responses.GET, + 'https://localhost/.well-known/nodeinfo', + status=404) + responses.add( + responses.GET, + 'https://example.com/user/mouse/outbox?page=true', + json={'orderedItems': []}, + status=200 + ) + + with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) # @responses.activate # def test_key_needs_refresh(self): @@ -172,26 +173,26 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_changed_data(self): - '''Message data must match the digest header.''' - response = self.send_test_request( - self.mouse, - send_data=get_follow_data(self.mouse, self.cat)) - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_changed_data(self): +# '''Message data must match the digest header.''' +# response = self.send_test_request( +# self.mouse, +# send_data=get_follow_data(self.mouse, self.cat)) +# self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_invalid_digest(self): - response = self.send_test_request( - self.mouse, - digest='SHA-256=AAAAAAAAAAAAAAAAAA') - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_invalid_digest(self): +# response = self.send_test_request( +# self.mouse, +# digest='SHA-256=AAAAAAAAAAAAAAAAAA') +# self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_old_message(self): - '''Old messages should be rejected to prevent replay attacks.''' - response = self.send_test_request( - self.mouse, - date=http_date(time.time() - 301) - ) - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_old_message(self): +# '''Old messages should be rejected to prevent replay attacks.''' +# response = self.send_test_request( +# self.mouse, +# date=http_date(time.time() - 301) +# ) +# self.assertEqual(response.status_code, 401) From 829615cdd729ac2537d216fb35cee3046faf1047 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:18:10 -0800 Subject: [PATCH 07/11] Fixes celery mocks on more signature unit tests --- bookwyrm/tests/test_signing.py | 113 +++++++++++++++++---------------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index d57a34392..05cb8ea09 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -111,55 +111,56 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) -# @responses.activate -# def test_key_needs_refresh(self): -# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') -# data = json.loads(datafile.read_bytes()) -# data['id'] = self.fake_remote.remote_id -# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key -# del data['icon'] # Avoid having to return an avatar. -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# responses.add( -# responses.GET, -# 'https://localhost/.well-known/nodeinfo', -# status=404) -# responses.add( -# responses.GET, -# 'https://example.com/user/mouse/outbox?page=true', -# json={'orderedItems': []}, -# status=200 -# ) -# -# # Second and subsequent fetches get a different key: -# new_private_key, new_public_key = create_key_pair() -# new_sender = Sender( -# self.fake_remote.remote_id, new_private_key, new_public_key) -# data['publicKey']['publicKeyPem'] = new_public_key -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# -# # Key correct: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) -# -# # Old key is cached, so still works: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) -# -# # Try with new key: -# response = self.send_test_request(sender=new_sender) -# self.assertEqual(response.status_code, 200) -# -# # Now the old key will fail: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 401) + @responses.activate + def test_key_needs_refresh(self): + datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') + data = json.loads(datafile.read_bytes()) + data['id'] = self.fake_remote.remote_id + data['publicKey']['publicKeyPem'] = self.fake_remote.public_key + del data['icon'] # Avoid having to return an avatar. + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + responses.add( + responses.GET, + 'https://localhost/.well-known/nodeinfo', + status=404) + responses.add( + responses.GET, + 'https://example.com/user/mouse/outbox?page=true', + json={'orderedItems': []}, + status=200 + ) + + # Second and subsequent fetches get a different key: + new_private_key, new_public_key = create_key_pair() + new_sender = Sender( + self.fake_remote.remote_id, new_private_key, new_public_key) + data['publicKey']['publicKeyPem'] = new_public_key + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + + with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _: + # Key correct: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Old key is cached, so still works: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Try with new key: + response = self.send_test_request(sender=new_sender) + self.assertEqual(response.status_code, 200) + + # Now the old key will fail: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 401) @responses.activate @@ -173,13 +174,13 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_changed_data(self): -# '''Message data must match the digest header.''' -# response = self.send_test_request( -# self.mouse, -# send_data=get_follow_data(self.mouse, self.cat)) -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_changed_data(self): + '''Message data must match the digest header.''' + response = self.send_test_request( + self.mouse, + send_data=get_follow_data(self.mouse, self.cat)) + self.assertEqual(response.status_code, 401) # @pytest.mark.integration # def test_invalid_digest(self): From a8f3ddec05db7c9ee766a4e670fb2843d1688424 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:39:33 -0800 Subject: [PATCH 08/11] Trying to avoid issues from execusing http requests --- bookwyrm/tests/test_signing.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 05cb8ea09..817e55890 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,8 +155,8 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: - response = self.send_test_request(sender=new_sender) - self.assertEqual(response.status_code, 200) +# response = self.send_test_request(sender=new_sender) +# self.assertEqual(response.status_code, 200) # Now the old key will fail: response = self.send_test_request(sender=self.fake_remote) @@ -177,17 +177,19 @@ class Signature(TestCase): @pytest.mark.integration def test_changed_data(self): '''Message data must match the digest header.''' - response = self.send_test_request( - self.mouse, - send_data=get_follow_data(self.mouse, self.cat)) - self.assertEqual(response.status_code, 401) + with patch('bookwyrm.remote_user.refresh_remote_user') as _: + response = self.send_test_request( + self.mouse, + send_data=get_follow_data(self.mouse, self.cat)) + self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_invalid_digest(self): -# response = self.send_test_request( -# self.mouse, -# digest='SHA-256=AAAAAAAAAAAAAAAAAA') -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_invalid_digest(self): + with patch('bookwyrm.remote_user.refresh_remote_user') as _: + response = self.send_test_request( + self.mouse, + digest='SHA-256=AAAAAAAAAAAAAAAAAA') + self.assertEqual(response.status_code, 401) # @pytest.mark.integration # def test_old_message(self): From f173d674ac383b677843bfeacef8d81ae864f42c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:53:38 -0800 Subject: [PATCH 09/11] Mock fetch_user function which makes http request --- bookwyrm/tests/test_signing.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 817e55890..e393062ed 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,12 +155,12 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: -# response = self.send_test_request(sender=new_sender) -# self.assertEqual(response.status_code, 200) + #response = self.send_test_request(sender=new_sender) + #self.assertEqual(response.status_code, 200) # Now the old key will fail: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 401) + #response = self.send_test_request(sender=self.fake_remote) + #self.assertEqual(response.status_code, 401) @responses.activate @@ -177,7 +177,7 @@ class Signature(TestCase): @pytest.mark.integration def test_changed_data(self): '''Message data must match the digest header.''' - with patch('bookwyrm.remote_user.refresh_remote_user') as _: + with patch('bookwyrm.remote_user.fetch_user_data') as _: response = self.send_test_request( self.mouse, send_data=get_follow_data(self.mouse, self.cat)) @@ -185,17 +185,18 @@ class Signature(TestCase): @pytest.mark.integration def test_invalid_digest(self): - with patch('bookwyrm.remote_user.refresh_remote_user') as _: + with patch('bookwyrm.remote_user.fetch_user_data') as _: response = self.send_test_request( self.mouse, digest='SHA-256=AAAAAAAAAAAAAAAAAA') self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_old_message(self): -# '''Old messages should be rejected to prevent replay attacks.''' -# response = self.send_test_request( -# self.mouse, -# date=http_date(time.time() - 301) -# ) -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_old_message(self): + '''Old messages should be rejected to prevent replay attacks.''' + with patch('bookwyrm.remote_user.fetch_user_data') as _: + response = self.send_test_request( + self.mouse, + date=http_date(time.time() - 301) + ) + self.assertEqual(response.status_code, 401) From 4ec557fc5dec8d75b0e634f3941c87e151b142f2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 14:15:13 -0800 Subject: [PATCH 10/11] fixes unit tests for incoming and outgoing follows --- bookwyrm/tests/incoming/test_follow.py | 54 +++++++------ bookwyrm/tests/outgoing/test_follow.py | 108 +++++++++++++------------ bookwyrm/tests/test_signing.py | 8 +- 3 files changed, 90 insertions(+), 80 deletions(-) diff --git a/bookwyrm/tests/incoming/test_follow.py b/bookwyrm/tests/incoming/test_follow.py index 377c70205..50e47d88b 100644 --- a/bookwyrm/tests/incoming/test_follow.py +++ b/bookwyrm/tests/incoming/test_follow.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, incoming @@ -18,29 +19,30 @@ class IncomingFollow(TestCase): self.local_user.save() -# def test_handle_follow(self): -# activity = { -# "@context": "https://www.w3.org/ns/activitystreams", -# "id": "https://example.com/users/rat/follows/123", -# "type": "Follow", -# "actor": "https://example.com/users/rat", -# "object": "http://local.com/user/mouse" -# } -# -# incoming.handle_follow(activity) -# -# # notification created -# notification = models.Notification.objects.get() -# self.assertEqual(notification.user, self.local_user) -# self.assertEqual(notification.notification_type, 'FOLLOW') -# -# # the request should have been deleted -# requests = models.UserFollowRequest.objects.all() -# self.assertEqual(list(requests), []) -# -# # the follow relationship should exist -# follow = models.UserFollows.objects.get(user_object=self.local_user) -# self.assertEqual(follow.user_subject, self.remote_user) + def test_handle_follow(self): + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/users/rat", + "object": "http://local.com/user/mouse" + } + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) + + # notification created + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.local_user) + self.assertEqual(notification.notification_type, 'FOLLOW') + + # the request should have been deleted + requests = models.UserFollowRequest.objects.all() + self.assertEqual(list(requests), []) + + # the follow relationship should exist + follow = models.UserFollows.objects.get(user_object=self.local_user) + self.assertEqual(follow.user_subject, self.remote_user) def test_handle_follow_manually_approved(self): @@ -55,7 +57,8 @@ class IncomingFollow(TestCase): self.local_user.manually_approves_followers = True self.local_user.save() - incoming.handle_follow(activity) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) # notification created notification = models.Notification.objects.get() @@ -81,7 +84,8 @@ class IncomingFollow(TestCase): "object": "http://local.com/user/nonexistent-user" } - incoming.handle_follow(activity) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) # do nothing notifications = models.Notification.objects.all() diff --git a/bookwyrm/tests/outgoing/test_follow.py b/bookwyrm/tests/outgoing/test_follow.py index a1ea3f690..89e677abf 100644 --- a/bookwyrm/tests/outgoing/test_follow.py +++ b/bookwyrm/tests/outgoing/test_follow.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, outgoing @@ -19,54 +20,59 @@ class Following(TestCase): ) -# def test_handle_follow(self): -# self.assertEqual(models.UserFollowRequest.objects.count(), 0) -# -# outgoing.handle_follow(self.local_user, self.remote_user) -# rel = models.UserFollowRequest.objects.get() -# -# self.assertEqual(rel.user_subject, self.local_user) -# self.assertEqual(rel.user_object, self.remote_user) -# self.assertEqual(rel.status, 'follow_request') -# -# -# def test_handle_unfollow(self): -# self.remote_user.followers.add(self.local_user) -# self.assertEqual(self.remote_user.followers.count(), 1) -# outgoing.handle_unfollow(self.local_user, self.remote_user) -# -# self.assertEqual(self.remote_user.followers.count(), 0) -# -# -# def test_handle_accept(self): -# rel = models.UserFollowRequest.objects.create( -# user_subject=self.local_user, -# user_object=self.remote_user -# ) -# rel_id = rel.id -# -# outgoing.handle_accept(rel) -# # request should be deleted -# self.assertEqual( -# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 -# ) -# # follow relationship should exist -# self.assertEqual(self.remote_user.followers.first(), self.local_user) -# -# -# def test_handle_reject(self): -# rel = models.UserFollowRequest.objects.create( -# user_subject=self.local_user, -# user_object=self.remote_user -# ) -# rel_id = rel.id -# -# outgoing.handle_reject(rel) -# # request should be deleted -# self.assertEqual( -# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 -# ) -# # follow relationship should not exist -# self.assertEqual( -# models.UserFollows.objects.filter(id=rel_id).count(), 0 -# ) + def test_handle_follow(self): + self.assertEqual(models.UserFollowRequest.objects.count(), 0) + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_follow(self.local_user, self.remote_user) + + rel = models.UserFollowRequest.objects.get() + + self.assertEqual(rel.user_subject, self.local_user) + self.assertEqual(rel.user_object, self.remote_user) + self.assertEqual(rel.status, 'follow_request') + + + def test_handle_unfollow(self): + self.remote_user.followers.add(self.local_user) + self.assertEqual(self.remote_user.followers.count(), 1) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_unfollow(self.local_user, self.remote_user) + + self.assertEqual(self.remote_user.followers.count(), 0) + + + def test_handle_accept(self): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + rel_id = rel.id + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_accept(rel) + # request should be deleted + self.assertEqual( + models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 + ) + # follow relationship should exist + self.assertEqual(self.remote_user.followers.first(), self.local_user) + + + def test_handle_reject(self): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + rel_id = rel.id + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_reject(rel) + # request should be deleted + self.assertEqual( + models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 + ) + # follow relationship should not exist + self.assertEqual( + models.UserFollows.objects.filter(id=rel_id).count(), 0 + ) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index e393062ed..a5039306b 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,12 +155,12 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: - #response = self.send_test_request(sender=new_sender) - #self.assertEqual(response.status_code, 200) + response = self.send_test_request(sender=new_sender) + self.assertEqual(response.status_code, 200) # Now the old key will fail: - #response = self.send_test_request(sender=self.fake_remote) - #self.assertEqual(response.status_code, 401) + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 401) @responses.activate From 9e48328e9e3285d5dc3dfe80a04c20ec063b9e60 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 14:18:45 -0800 Subject: [PATCH 11/11] Mocks broadcast task for outgoing shelve tests --- bookwyrm/tests/outgoing/test_shelving.py | 78 +++++++++++++----------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/bookwyrm/tests/outgoing/test_shelving.py b/bookwyrm/tests/outgoing/test_shelving.py index c26bec84f..5567784e5 100644 --- a/bookwyrm/tests/outgoing/test_shelving.py +++ b/bookwyrm/tests/outgoing/test_shelving.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, outgoing @@ -25,39 +26,44 @@ class Shelving(TestCase): ) -# def test_handle_shelve(self): -# outgoing.handle_shelve(self.user, self.book, self.shelf) -# # make sure the book is on the shelf -# self.assertEqual(self.shelf.books.get(), self.book) -# -# -# def test_handle_shelve_to_read(self): -# shelf = models.Shelf.objects.get(identifier='to-read') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_shelve_reading(self): -# shelf = models.Shelf.objects.get(identifier='reading') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_shelve_read(self): -# shelf = models.Shelf.objects.get(identifier='read') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_unshelve(self): -# self.shelf.books.add(self.book) -# self.shelf.save() -# self.assertEqual(self.shelf.books.count(), 1) -# outgoing.handle_unshelve(self.user, self.book, self.shelf) -# self.assertEqual(self.shelf.books.count(), 0) + def test_handle_shelve(self): + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, self.shelf) + # make sure the book is on the shelf + self.assertEqual(self.shelf.books.get(), self.book) + + + def test_handle_shelve_to_read(self): + shelf = models.Shelf.objects.get(identifier='to-read') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_shelve_reading(self): + shelf = models.Shelf.objects.get(identifier='reading') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_shelve_read(self): + shelf = models.Shelf.objects.get(identifier='read') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_unshelve(self): + self.shelf.books.add(self.book) + self.shelf.save() + self.assertEqual(self.shelf.books.count(), 1) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_unshelve(self.user, self.book, self.shelf) + self.assertEqual(self.shelf.books.count(), 0)