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

Add tests for registration #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

euav
Copy link

@euav euav commented Sep 16, 2023

No description provided.

Copy link

@Romanskripko Romanskripko left a comment

Choose a reason for hiding this comment

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

Я сперва тоже попытался сделать по образу и подобию кода из лекции, но чёт быстро там всё причесать и куда надо положить у меня не вышло). Не уверен, что эти тесты рабочие, но выглядит всё здорово

'date_of_birth': mf('datetime.date'),
'job_title': mf('person.occupation'),
'phone': mf('person.telephone'),
'phone_type': mf('choice', items=[1, 2, 3]),

Choose a reason for hiding this comment

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

А это поле было в лекции, но в проекте такого поля у User'а нет. Вроде в assert_correct_user должны споткнуться об это, но если тесты проходят - стоит обратить внимание на это

Copy link

@alexbarev alexbarev left a comment

Choose a reason for hiding this comment

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

I recommend using linters consistently. Check for plugins in your IDE that can highlight code issues and offer automatic fixes.

Understand how to globally silence certain linter errors. This way, you won't overuse type: ignore, especially if you choose not to add docstrings to every function.

Lastly, consider writing a shell script to automatically check with linters like flake8, pylint, and mypy or manually run them from IDE integrated shell.

PS. Sorry for the accidental approval.



@final
class RegistrationData(UserData, total=False):

Choose a reason for hiding this comment

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

suggestion: Remove total=false

By default, all attributes are required. Isn't this the case with password1 and password2?

Suggested change
class RegistrationData(UserData, total=False):
class RegistrationData(UserData):

'phone_type': mf('choice', items=[1, 2, 3]),
}, iterations=1)
return {
**next(schema), # type: ignore[typeddict-item]
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.

praise: Using next(schema) is much cleaner than mine schema.create()[0]

Does it work fine? =)

@@ -22,5 +22,5 @@ class FavouritePicture(TimedMixin, models.Model):
url = models.URLField()

def __str__(self) -> str:
"""Beatuful representation."""
"""Beautiful representation."""

Choose a reason for hiding this comment

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

note: If you're using VSCode, there's a plugin called 'Code Spell Checker'.

It's useful for catching typos, though it can produce false errors with variable names.

"""User registration data factory."""


@pytest.fixture(scope="session")
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.

polish: Please use either single or double quotes consistently.

Linters typically flag such inconsistencies. I noticed you used single quotes below. Plugins can assist with this, and you can even set up auto-formatting on save to automate this.

There are many minor improvements in the code quality that can be made automatically using linters at no extra cost.

Suggested change
@pytest.fixture(scope="session")
@pytest.fixture(scope='session')

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