-
Notifications
You must be signed in to change notification settings - Fork 3
добавлены тест на общий лимит комментариев от одного пользователя и т… #179
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -792,3 +792,114 @@ def test_post_like(client, dbsession, comment): | |
| dbsession.refresh(comment) | ||
| assert comment.like_count == 0 | ||
| assert comment.dislike_count == 0 | ||
|
|
||
|
|
||
| def test_comment_lecturer_limit( | ||
| client, | ||
| lecturers, | ||
| authlib_user, | ||
| mocker, | ||
| ): | ||
| """ | ||
| Тест лимита на одного лектора | ||
| """ | ||
| new_user = authlib_user.copy() | ||
| new_user["id"] = 99999 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Зачем менять id юзера? Кажется не за чем, тесты запускаются изолированно, это ни на что не влияет. |
||
|
|
||
| achive_get_url = settings.API_URL + f"achievement/user/{new_user.get('id')}" | ||
| achive_post_url = ( | ||
| settings.API_URL | ||
| + f"achievement/achievement/{settings.FIRST_COMMENT_ACHIEVEMENT_ID}/reciever/{new_user.get('id')}" | ||
| ) | ||
| mock_aiohttp_session = aiohttp_mock( | ||
| authlib_user_id=new_user.get("id"), | ||
| aiohttp_response_status=status.HTTP_200_OK, | ||
| achievement_id=settings.FIRST_COMMENT_ACHIEVEMENT_ID, | ||
| get_url=achive_get_url, | ||
| post_url=achive_post_url, | ||
| ) | ||
| mocker.patch("aiohttp.ClientSession", return_value=mock_aiohttp_session) | ||
|
|
||
| lecturer_id = lecturers[0].id | ||
| body = { | ||
| "subject": "Subject", | ||
| "text": "Text", | ||
| "mark_kindness": 1, | ||
| "mark_freebie": 0, | ||
| "mark_clarity": 0, | ||
| } | ||
|
|
||
| for _ in range(settings.COMMENT_TO_LECTURER_LIMIT - 1): | ||
| response = client.post(url, json=body, params={"lecturer_id": lecturer_id}) | ||
|
petrCher marked this conversation as resolved.
|
||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| response_5 = client.post(url, json=body, params={"lecturer_id": lecturer_id}) | ||
| assert response_5.status_code == status.HTTP_200_OK | ||
|
|
||
| response_6 = client.post(url, json=body, params={"lecturer_id": lecturer_id}) | ||
| assert response_6.status_code == status.HTTP_429_TOO_MANY_REQUESTS | ||
|
|
||
|
|
||
| def test_comment_total_limit( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Так же, если данные в БД создаются в рамках теста, обязательно нужно делать очистку. Чтобы тест был чище, лучше сделать фикстру, которая создает нужные данные и автоматически очищает их после yield. |
||
| client, | ||
| dbsession, | ||
| mocker, | ||
| ): | ||
| """ | ||
| Тест общего лимита комментариев пользователя за период | ||
| """ | ||
| dbsession.query(LecturerUserComment).delete() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Если это очистка после предыдущего теста, то она не будет работать корректно. Да, в рамках одного файла тесты запускаются в порядке их объявления, но могут быть и другие файлы(test_lecturer.py). Не стоит полагаться на этот порядок. Очистку лучше проводить в том же тесте, в котором были созданы объекты, чтобы тесты были независимы(или в фикстуре, см. ниже)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. да, всю работу с бд лучше выносить в фикстуры, в самих тестах только вызов эндпоинтов и проверка логики работы |
||
| dbsession.query(Comment).delete() | ||
| dbsession.commit() | ||
|
|
||
| from rating_api.models import Lecturer | ||
|
|
||
| extra_lecturers = [] | ||
| for i in range(5): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Хорошо, когда создание данных для теста и логика проверки в тесте разделены. Здесь мне кажется лучше создать отдельную фикстуру для создания лекторов. То же касается и комментариев. Подправить уже существующу фикстуру немножко затруднительно, потому что много тестов в test_lecturer.py зависят текущего количества лекоторов в фикстуре lecturers и даже при добавлении одного их придется править.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. да, надо фикстуру создать, которую в текущем тесте просто вызовем |
||
| lecturer = Lecturer( | ||
| id=200 + i, | ||
| first_name=f"total_fname{i}", | ||
| last_name=f"total_lname{i}", | ||
| middle_name=f"total_mname{i}", | ||
| timetable_id=5000 + i, | ||
| ) | ||
| dbsession.add(lecturer) | ||
| extra_lecturers.append(lecturer) | ||
| dbsession.commit() | ||
|
|
||
| for lecturer in extra_lecturers: | ||
| dbsession.refresh(lecturer) | ||
|
|
||
| new_user = {"id": 99999, "email": "test@example.com"} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Так же не понимаю, как пользователь с другим id и почтой влияет на логику проверки лимита комментариев. Помимо этого, хардкодить тестовые данные в тесте не очень хорошо, тест становистся хрупким. Для этого как раз есть фикстура authlib_user. |
||
|
|
||
| achive_get_url = settings.API_URL + f"achievement/user/{new_user.get('id')}" | ||
| achive_post_url = ( | ||
| settings.API_URL | ||
| + f"achievement/achievement/{settings.FIRST_COMMENT_ACHIEVEMENT_ID}/reciever/{new_user.get('id')}" | ||
| ) | ||
| mock_aiohttp_session = aiohttp_mock( | ||
| authlib_user_id=new_user.get("id"), | ||
| aiohttp_response_status=status.HTTP_200_OK, | ||
| achievement_id=settings.FIRST_COMMENT_ACHIEVEMENT_ID, | ||
| get_url=achive_get_url, | ||
| post_url=achive_post_url, | ||
| ) | ||
| mocker.patch("aiohttp.ClientSession", return_value=mock_aiohttp_session) | ||
|
|
||
| body = { | ||
| "subject": "TestSubject", | ||
| "text": "TestText", | ||
| "mark_kindness": 1, | ||
| "mark_freebie": 0, | ||
| "mark_clarity": 0, | ||
| } | ||
|
|
||
| # По 4 коммента каждому из 5 лекторов = 20 | ||
| for lecturer in extra_lecturers: | ||
| for _ in range(4): | ||
| response = client.post(url, json=body, params={"lecturer_id": lecturer.id}) | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| # 21й - превышение лимита | ||
| response_21 = client.post(url, json=body, params={"lecturer_id": extra_lecturers[0].id}) | ||
| assert response_21.status_code == status.HTTP_429_TOO_MANY_REQUESTS | ||
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.
Нет очистки БД от созданных объектов комментариев. Очистку необходимо проводить, чтобы обеспечить независимость запуска тестов. Сейчас несколько тестов из test_lecturer.py падают, потому что в БД
остались лишние комменты:
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.
https://github.com/profcomff/rating-api/actions/runs/33264837617/job/99132955286?pr=179

здесь подробный вывод в print report