Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Lesson1 updates #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

AlexKupreev
Copy link

No description provided.

Copy link

@alkurbatov alkurbatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В целом все хорошо. Жаль, что не получилось написать больше тестов.

@@ -66,6 +66,7 @@ dennis = "^1.1"
dump-env = "^1.3"
ipython = "^8.15"
import-linter = "^1.11"
mimesis = "^11.1.0"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Минорный момент: лучше не продуктовые зависимости добавлять в dev группу.
Например, это можно сделать командой poetry add --group dev xxx или вручную переставить в файле.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

оно вроде и есть в dev-зависимостях?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Точно, проглядел.
My bad :(

email,
password,
)
client.force_login(user)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот этот юзер для апдейта классно бы смотрелся в фикстуре.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

верно, спасибо!

response = client.get('/pictures/dashboard')
assert response.status_code == HTTPStatus.OK

response = client.get('/pictures/favourites')
Copy link

@alkurbatov alkurbatov Sep 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут чтобы меньше повторяться, можно применить pytest.mark.parametrize, например:

@pytest.mark.parametrize('endpoint', [
    '/pictures/dashboard',
    '/pictures/favourites',
])
def test_picture_pages_authorized(
    client: Client,
    django_user_model: User,
    endpoint: str,
) -> None:
...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

согласен, спасибо!

@@ -2,6 +2,11 @@

import pytest
from django.test import Client
from plugins.identity.user import (
ProfileAssertion,
ProfileDataFactory,
Copy link

@alexbarev alexbarev Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: If we treat plugins as standalone modules, will this affect test or CI duration?

As @sobolevn suggested, we could remove __init__.py from the plugin subdirectories. We should then import classes only for type checking under the TYPE_CHECKING condition.

If plugins become significantly large, will this change influence the startup time or CI duration?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, TYPE_CHECKING was used in the presented code examples for the case of mypy type check - if mypy is not used, switching them off may speed up a bit.

I'm not really sure about __init__.py and auto-importing, but afaik they are imported only once per execution, not sure that might save a lot of time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants