Skip to content

Feature/fix some test and do level 2#5

Open
Beloded1 wants to merge 4 commits intomainfrom
feature/fix_some_test_and_do_level_2
Open

Feature/fix some test and do level 2#5
Beloded1 wants to merge 4 commits intomainfrom
feature/fix_some_test_and_do_level_2

Conversation

@Beloded1
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@vppuzakov vppuzakov left a comment

Choose a reason for hiding this comment

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

👍 вмерживай, по комментам в некст ПР поправь в основном по мокам в 4м и 5м тестах

Comment thread .coverage
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 добавить в .gitignore и удалить из репозитория

Comment thread requirements.txt
mypy==1.3.0
freezegun==1.2.2 No newline at end of file
freezegun==1.2.2
unittest2==1.1.0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 эта библиотека лишняя https://pypi.org/project/unittest2/, unittest есть из коробки

Comment thread setup.cfg
[flake8]
max-line-length = 134
ignore = D100, D103, WPS116, WPS118
inline-quotes = ',"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 должна быть какая-то одна либо одинарная либо двойная, лучше
# We also support "double" and "single"

Comment thread setup.cfg
--testdox

[flake8]
max-line-length = 134
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 максимальная позволительная длина 119, давай сделаем 99 или 89

Comment thread tests/conftest.py
return Expense(
amount=decimal.Decimal(amount),
currency=Currency.RUB,
card=card or BankCard(last_digits="1234", owner="Alexandr"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 можно еще сделать фикстуру-фабрику (если понадобится потом) и использовать ее make_bankcard

mock_config_parser.__getitem__.return_value = {'field_name': 'value'}
result = fetch_app_config_field('config.ini', 'field_name')
expected_result = 'value'
assert result == expected_result
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 тут переменные как будто роли не играют - можно сразу написать:

assert fetch_app_config_field('config.ini', 'field_name') == 'value'



def test__fetch_extra_fields_configuration__get_app_config_field_without_value_return_none(mock_config_parser):
mock_config_parser.__getitem__.return_value = {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 сделай мок-фикстуру на метод getitem конфигпарсера и используй ее.

mock_config_parser.__getitem__.return_value = {}
result = fetch_app_config_field('config.ini', 'field_name')
expected_result = None
assert result == expected_result
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 в таком коротком тесте используй сразу вызов и assert для читаемости


@pytest.mark.parametrize(
'expense, ExpenseCategory',
'spent_in, expected_category',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍 вот такая фикстура с простыми типами хорошая

ExpenseCategory.ONLINE_SUBSCRIPTIONS),
(make_expenses(spent_in='Wonder pharm'),
ExpenseCategory.MEDICINE_PHARMACY),
('Bastard place', ExpenseCategory.BAR_RESTAURANT),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍 вот такая фикстура с простыми типами хорошая

Copy link
Copy Markdown

@vppuzakov vppuzakov left a comment

Choose a reason for hiding this comment

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

👍 вмерживай, по комментам в некст ПР поправь в основном по мокам в 4м и 5м тестах

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants