Conversation
taaylor
force-pushed
the
feat/classroom-filtering
branch
from
a563d55 to
4520c4e
Compare
taaylor
added
ci:covered
| kind = CLASSROOMS_KINDS[ | ||
| i // (CLASSROOMS_LIST_SIZE // 2) % len(CLASSROOMS_KINDS) | ||
| ] | ||
| match kind: | ||
| case ClassroomKind.INDIVIDUAL: | ||
| yield await IndividualClassroom.create( | ||
| **factories.IndividualClassroomInputFactory.build_python( | ||
| subject_id=CLASSROOMS_SUBJECT_IDS[ | ||
| i % len(CLASSROOMS_SUBJECT_IDS) | ||
| ], | ||
| ), | ||
| status=CLASSROOMS_STATUSES[i % len(CLASSROOMS_STATUSES)], | ||
| tutor_id=tutor_user_id, | ||
| student_id=tutor_user_id + i + 1, | ||
| tutor_name=faker.name(), | ||
| student_name=faker.name(), | ||
| created_at=last_created_at, | ||
| ) | ||
| case ClassroomKind.GROUP: | ||
| yield await GroupClassroom.create( | ||
| **factories.GroupClassroomInputFactory.build_python( | ||
| subject_id=CLASSROOMS_SUBJECT_IDS[ | ||
| i % len(CLASSROOMS_SUBJECT_IDS) | ||
| ], | ||
| ), | ||
| status=CLASSROOMS_STATUSES[i % len(CLASSROOMS_STATUSES)], |
There was a problem hiding this comment.
issue(repeat)(repeat): генерация данных неверная, полное покрытие не создаётся https://discord.com/channels/706806130348785715/1423620574089052244/1439681556489834778
| classroom_requests_parametrization = pytest.mark.parametrize( | ||
| ("offset", "limit"), | ||
| [ | ||
| pytest.param(None, CLASSROOMS_LIST_SIZE, id="start_to_end"), | ||
| pytest.param(None, CLASSROOMS_LIST_SIZE // 2, id="start_to_middle"), | ||
| pytest.param( | ||
| CLASSROOMS_LIST_SIZE // 2, | ||
| CLASSROOMS_LIST_SIZE, | ||
| id="middle_to_end", | ||
| ), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
polish: некорректный порядок (создавать надо прямо перед первым использованием, не в рандомном месте файла)
| (None, {ClassroomKind.INDIVIDUAL.value}), | ||
| (None, {ClassroomStatus.ACTIVE.value}), | ||
| (None, {CLASSROOMS_SUBJECT_IDS[0]}), |
There was a problem hiding this comment.
issue(repeat): хардкод, не рандом
| pytest.param(None, set(CLASSROOMS_STATUSES[:2]), None, id="multiple_statuses"), | ||
| pytest.param(set(CLASSROOMS_KINDS[:2]), None, None, id="multiple_kinds"), | ||
| pytest.param( | ||
| None, None, set(CLASSROOMS_SUBJECT_IDS[:2]), id="multiple_subject_ids" | ||
| ), | ||
| pytest.param(set(CLASSROOMS_KINDS), None, None, id="all_kinds"), | ||
| pytest.param(None, set(CLASSROOMS_STATUSES), None, id="all_statuses"), | ||
| pytest.param( | ||
| set(CLASSROOMS_KINDS), | ||
| set(CLASSROOMS_STATUSES), | ||
| set(CLASSROOMS_SUBJECT_IDS[:2]), | ||
| id="all_filter", | ||
| ), |
There was a problem hiding this comment.
issue(repeat): тоже хардкод, не рандом
There was a problem hiding this comment.
Проще будет сделать это отдельным тестом, тогда и рандомизация получится внутри теста (как и должно быть), и параметризация будет не такая страшная
| set(CLASSROOMS_KINDS), | ||
| set(CLASSROOMS_STATUSES), | ||
| set(CLASSROOMS_SUBJECT_IDS[:2]), | ||
| id="all_filter", |
There was a problem hiding this comment.
issue: это не фильтр всего и это не так переводится на английский
| kind = CLASSROOMS_KINDS[ | ||
| i | ||
| // (CLASSROOMS_LIST_SIZE // len(CLASSROOMS_KINDS)) | ||
| % len(CLASSROOMS_KINDS) | ||
| ] | ||
| match kind: | ||
| case ClassroomKind.INDIVIDUAL: | ||
| yield await IndividualClassroom.create( | ||
| **factories.IndividualClassroomInputFactory.build_python( | ||
| subject_id=CLASSROOMS_SUBJECT_IDS[ | ||
| (i // len(CLASSROOMS_STATUSES)) | ||
| % len(CLASSROOMS_SUBJECT_IDS) | ||
| ], | ||
| ), | ||
| status=CLASSROOMS_STATUSES[i % len(CLASSROOMS_STATUSES)], | ||
| tutor_id=student_user_id + i + 1, | ||
| student_id=student_user_id, | ||
| tutor_name=faker.name(), | ||
| student_name=faker.name(), | ||
| created_at=last_created_at, | ||
| ) | ||
| case ClassroomKind.GROUP: | ||
| group_classroom = await GroupClassroom.create( | ||
| **factories.GroupClassroomInputFactory.build_python( | ||
| subject_id=CLASSROOMS_SUBJECT_IDS[ | ||
| (i // len(CLASSROOMS_STATUSES)) | ||
| % len(CLASSROOMS_SUBJECT_IDS) | ||
| ], | ||
| ), | ||
| status=CLASSROOMS_STATUSES[i % len(CLASSROOMS_STATUSES)], | ||
| tutor_id=student_user_id + i + 1, | ||
| created_at=last_created_at, | ||
| ) | ||
| await Enrollment.create( | ||
| group_classroom_id=group_classroom.id, | ||
| student_id=student_user_id, | ||
| ) | ||
| yield group_classroom |
There was a problem hiding this comment.
suggestion: очень много одинакового в студенте и репетиторе, стоит чутка подобъединить, например, через общую функцию или фикстуру, которая чисто генерируется данные (kind, status, subject), но не создаёт ничего в БД
| filtered_tutor_classroom = [ | ||
| classroom | ||
| for classroom in tutor_classrooms | ||
| if all( | ||
| ( | ||
| statuses is None or classroom.status in statuses, | ||
| kinds is None or classroom.kind in kinds, | ||
| subject_ids is None or classroom.subject_id in subject_ids, | ||
| cursor is None or classroom.created_at < cursor.created_at, | ||
| ) | ||
| ) | ||
| ] |
There was a problem hiding this comment.
issue(repeat): некорректный нейминг переменной
Плюс некорректная типизация
| CLASSROOMS_LIST_SIZE = 8 | ||
| CLASSROOMS_STATUSES = list(ClassroomStatus) | ||
| CLASSROOMS_KINDS = list(ClassroomKind) | ||
| CLASSROOMS_SUBJECT_IDS = [*random.sample(range(1, 999), k=2), None] |
There was a problem hiding this comment.
issue: рандом в глобальном скоупе. Должен быть только в тестах и фикстурах
issue: нет типизации (важно из-за None)
| CLASSROOMS_LIST_SIZE = ( | ||
| len(CLASSROOMS_SUBJECT_IDS) * len(CLASSROOMS_KINDS) * len(CLASSROOMS_STATUSES) | ||
| ) |
There was a problem hiding this comment.
issue: некорректный размер датасета, нет константы
There was a problem hiding this comment.
| classroom_requests_filter = pytest.mark.parametrize( | ||
| ("kinds", "statuses", "subject_ids"), | ||
| [ |
There was a problem hiding this comment.
2 participants
https://github.com/xi-effect/xi.progress/issues/9