добавлены тест на общий лимит комментариев от одного пользователя и т… by businkv · Pull Request #179 · profcomff/rating-api

businkv · 2026-08-29T17:08:14Z

Добавлен тест на общий лимит комментов от одного пользователя и тест на лимит комментов одному лектору

Изменения

Детали реализации

Check-List

Вы проверили свой код перед отправкой запроса?
Вы написали тесты к реализованным функциям?
Вы не забыли применить форматирование black и isort для Back-End или Prettier для Front-End?

…ест на лимит комментариев для одного лектора

github-actions · 2026-08-29T17:09:52Z

Coverage Report

File	Stmts	Miss	Cover	Missing
rating_api
__main__.py	4	4	0%	1–6
exceptions.py	43	1	98%	58
rating_api/models
base.py	64	4	94%	24–27
db.py	162	18	89%	114, 116, 118, 120, 131, 170, 188, 198, 212, 221–223, 257, 271–283
rating_api/routes
base.py	16	1	94%	40
comment.py	151	25	83%	191, 264–265, 267–268, 276–281, 290, 297–299, 329, 352, 392–403, 430
exc_handlers.py	32	1	97%	50
lecturer.py	109	9	92%	71–72, 82, 88–89, 229, 237, 259, 265
rating_api/schemas
base.py	12	4	67%	6–9
models.py	155	3	98%	189, 191, 201
rating_api/utils
mark.py	8	8	0%	1–19
TOTAL	790	78	90%

Summary

Tests	Skipped	Failures	Errors	Time
96	0 💤	9 ❌	0 🔥	22.247s ⏱️

CaseAsLimbo · 2026-08-30T12:48:35Z

+    Тест лимита на одного лектора
+    """
+    new_user = authlib_user.copy()
+    new_user["id"] = 99999


Зачем менять id юзера? Кажется не за чем, тесты запускаются изолированно, это ни на что не влияет.

CaseAsLimbo · 2026-08-30T13:10:03Z

    assert comment.dislike_count == 0
+
+
+def test_comment_lecturer_limit(


Нет очистки БД от созданных объектов комментариев. Очистку необходимо проводить, чтобы обеспечить независимость запуска тестов. Сейчас несколько тестов из test_lecturer.py падают, потому что в БД

остались лишние комменты:

https://github.com/profcomff/rating-api/actions/runs/33264837617/job/99132955286?pr=179
здесь подробный вывод в print report

CaseAsLimbo · 2026-08-30T13:15:13Z

+    """
+    Тест общего лимита комментариев пользователя за период
+    """
+    dbsession.query(LecturerUserComment).delete()


Если это очистка после предыдущего теста, то она не будет работать корректно. Да, в рамках одного файла тесты запускаются в порядке их объявления, но могут быть и другие файлы(test_lecturer.py). Не стоит полагаться на этот порядок. Очистку лучше проводить в том же тесте, в котором были созданы объекты, чтобы тесты были независимы(или в фикстуре, см. ниже)

да, всю работу с бд лучше выносить в фикстуры, в самих тестах только вызов эндпоинтов и проверка логики работы

CaseAsLimbo · 2026-08-30T13:18:50Z

+    for lecturer in extra_lecturers:
+        dbsession.refresh(lecturer)
+
+    new_user = {"id": 99999, "email": "test@example.com"}


Так же не понимаю, как пользователь с другим id и почтой влияет на логику проверки лимита комментариев. Помимо этого, хардкодить тестовые данные в тесте не очень хорошо, тест становистся хрупким. Для этого как раз есть фикстура authlib_user.

CaseAsLimbo · 2026-08-30T13:28:30Z

+    from rating_api.models import Lecturer
+
+    extra_lecturers = []
+    for i in range(5):


Хорошо, когда создание данных для теста и логика проверки в тесте разделены. Здесь мне кажется лучше создать отдельную фикстуру для создания лекторов. То же касается и комментариев. Подправить уже существующу фикстуру немножко затруднительно, потому что много тестов в test_lecturer.py зависят текущего количества лекоторов в фикстуре lecturers и даже при добавлении одного их придется править.

да, надо фикстуру создать, которую в текущем тесте просто вызовем

CaseAsLimbo · 2026-08-30T13:31:26Z

+    assert response_6.status_code == status.HTTP_429_TOO_MANY_REQUESTS
+
+
+def test_comment_total_limit(


Так же, если данные в БД создаются в рамках теста, обязательно нужно делать очистку. Чтобы тест был чище, лучше сделать фикстру, которая создает нужные данные и автоматически очищает их после yield.

CaseAsLimbo · 2026-08-30T14:06:43Z

В данных тестах мы проверяем логику бщего лимита комментариев и лимита комментариев на одного лектора. Соответвенно у нас есть как миниму 4 основных кейса: лимит на лекторов не превышен/превышен, общий лимит не превышен/превышен. Эти четыре кейса мы можем удобно описать в параметризации теста(@pytest.mark.parametrize). А данные создавать с помощью фикстуры-фабрики(то есть фикстуры возвращающей фукнцию, создающую переданное через параметризацию теста количество комментариев или лекторов). А очистку проводить после yield в этой же фикстуре. Так тест будет чистым, данные будут создаваться отдельно в фикстуре и корректно удаляться после каждого теста.

CaseAsLimbo · 2026-08-30T14:09:04Z

Так же возможно мне стоит отметить, что название ветки не информативно(менять его не надо, потому что это вроде как приведет к закрытию PR, но можно в будущем делать более информативное название)
https://conventionalbranch.org/

petrCher

от себя еще добавлю, что захардкоженные числа по типу 20, 21 и тд что в коде и в названии переменных присутствуют - не очень хорошая практика, ориентируемся на поля для максимальных комментариев из settings, относительно них работаем, так как настоящие значения могут отличаться от дефолтных

понимаю что в тестах можно и дефолтные взять и по сути ни на что не повлияет, но если поменять дефолтное значение то логика может рухнуть)

petrCher · 2026-09-01T11:29:34Z

+    """
+    Тест общего лимита комментариев пользователя за период
+    """
+    dbsession.query(LecturerUserComment).delete()


да, всю работу с бд лучше выносить в фикстуры, в самих тестах только вызов эндпоинтов и проверка логики работы

petrCher · 2026-09-01T11:30:52Z

+    from rating_api.models import Lecturer
+
+    extra_lecturers = []
+    for i in range(5):


да, надо фикстуру создать, которую в текущем тесте просто вызовем

petrCher · 2026-09-01T11:46:02Z

насчет названия ветки да, лучше более информативное название давать)

petrCher · 2026-09-01T11:48:02Z

все комменты максима, которые отметил лайком надо переделать

просмотрел бегло пока что, когда все будет исправлено внимательно изучу
единственный вопрос @businkv не совсем понял почему тесты на лимит создания не внесены в тесты на создание коммента просто, которые уже готовы

CaseAsLimbo · 2026-09-02T04:36:18Z

Немножко добавлю, логику в ручке можно условно поделить на ту, что работает атомарно между вызовами с теми же параметрами, и ту что меняет поведение ручки между вызовами с теми же параметрами. Логика лимита относится к второй категории, потому что ручка создаёт комменты. Часто в тестах это решается множественным вызовом эндпоинта или созданием нескольких фикстур с разными наборами данных. Но при таком подходе нам потребуется создавать фикстуры под каждый кейс. Если же мы используем множественный вызов эндпоинта, то мы не сможем вплести эту проверку в общий тест с атомарной логикой. Решением здесь является использование фикстуры-фабрики вместо обычной - так создание данных становится управляемым через параметризацию. Мы явно задаём состояние БД по комментам в каждом кейсе.

businkv added 2 commits August 29, 2026 20:04

добавлены тест на общий лимит комментариев от одного пользователя и т…

18a8a87

…ест на лимит комментариев для одного лектора

формат

7ff4de0

businkv requested a review from petrCher August 29, 2026 17:15

petrCher requested a review from CaseAsLimbo August 29, 2026 17:18

businkv assigned CaseAsLimbo and unassigned CaseAsLimbo Aug 29, 2026

CaseAsLimbo reviewed Aug 30, 2026

View reviewed changes

Comment thread tests/test_routes/test_comment.py

CaseAsLimbo reviewed Aug 30, 2026

View reviewed changes

petrCher requested changes Sep 1, 2026

View reviewed changes

		assert comment.dislike_count == 0


		def test_comment_lecturer_limit(

		assert response_6.status_code == status.HTTP_429_TOO_MANY_REQUESTS


		def test_comment_total_limit(

Uh oh!

Conversation

businkv commented Aug 29, 2026

Изменения

Детали реализации

Check-List

Uh oh!

github-actions Bot commented Aug 29, 2026

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CaseAsLimbo Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CaseAsLimbo Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CaseAsLimbo commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CaseAsLimbo commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrCher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

petrCher commented Sep 1, 2026

Uh oh!

petrCher commented Sep 1, 2026

Uh oh!

CaseAsLimbo commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CaseAsLimbo Aug 30, 2026 •

edited

Loading

CaseAsLimbo Aug 30, 2026 •

edited

Loading

CaseAsLimbo commented Aug 30, 2026 •

edited

Loading

CaseAsLimbo commented Aug 30, 2026 •

edited

Loading

CaseAsLimbo commented Sep 2, 2026 •

edited

Loading