-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat/dev add document download view #71
Conversation
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.
в целом ок, ничего критичного
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.
В целом ок.
Есть что поправить.
По цифрам на твое усмотрение.
makedoc/tests/conftest.py
Outdated
from rest_framework.test import APIClient | ||
from rest_framework_simplejwt.tokens import RefreshToken | ||
|
||
from users.models import CustomUser | ||
|
||
fake = Faker() |
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.
Создавать его нет необходимости, можно использовать имеющуюся фикстуру faker
makedoc/tests/conftest.py
Outdated
|
||
|
||
@pytest.fixture | ||
def test_data(): |
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.
фикстура-фабрика должна начинаться с make_, желательно переименовать
makedoc/tests/conftest.py
Outdated
@pytest.fixture | ||
def test_data(): | ||
return { | ||
"delivery_type": fake.random_element(elements=("auto", "manual")), |
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.
Значения которые мы передаем - ("auto", "rw", "self-deliery")
makedoc/tests/conftest.py
Outdated
"client_id": fake.random_int(min=1, max=100), | ||
"items": [ | ||
{"product_id": fake.random_int(min=1, max=100), | ||
"quantity": fake.random_int(min=1, max=10), |
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.
Количество у нас в тоннах, это FloatField также в сериализаторе.
10 - слишком мало, здесь можно и 1000 поставить.
makedoc/tests/conftest.py
Outdated
"items": [ | ||
{"product_id": fake.random_int(min=1, max=100), | ||
"quantity": fake.random_int(min=1, max=10), | ||
"discount": fake.random_int(min=0, max=50)}, |
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.
До 100 можно в тестах проверять. Может быть даже разнести на несколько кейсов, с 0, 100 и всеми остальными вариантами.
makedoc/tests/conftest.py
Outdated
"quantity": fake.random_int(min=1, max=10), | ||
"discount": fake.random_int(min=0, max=50)}, | ||
], | ||
"factory_id": fake.random_int(min=1, max=100), |
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.
Фабрик пока у нас 3, 100 слишком много.
makedoc/tests/conftest.py
Outdated
"discount": fake.random_int(min=0, max=50)}, | ||
], | ||
"factory_id": fake.random_int(min=1, max=100), | ||
"destination": fake.city(), |
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.
в destination мы получаем либо "0", либо две склеенных через запятую строки (можно и 2 pystr).
Наверное здесь лучше разнести тесты на 2 кейса.
makedoc/tests/conftest.py
Outdated
], | ||
"factory_id": fake.random_int(min=1, max=100), | ||
"destination": fake.city(), | ||
"delivery_cost": fake.random_int(min=100, max=2000), |
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.
В сериализаторе у нас минимальная стоимость рейса 0, максимальная на практике (пока) доходит до 15000.
Тоже наверное лучше на разные кейсы разнести: 0, все остальное.
No description provided.