Skip to content
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

Степан Заколюкин #32

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

Conversation

StepanZakolukin
Copy link

@StepanZakolukin StepanZakolukin commented Nov 6, 2024

@Inreek

…ектах: при добавлении новых полей в класс Person не придется вносить изменений в код тестирующего метода; кроме того для сравнения объектов Person используется более читаемый синтаксис
…ории ошибок, которые они проверяют, удалил повторяющиеся тесты, дополнил решение класс своими тестами
@StepanZakolukin StepanZakolukin changed the title Task refactoring tests Степан Заколюкин Nov 6, 2024
Comment on lines 17 to 20
// if (precision <= 0)
// throw new ArgumentException("precision must be a positive number");
// if (scale < 0 || scale >= precision)
// throw new ArgumentException("precision must be a non-negative number less or equal than precision");
Copy link

Choose a reason for hiding this comment

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

проверки нужны

public void NumberValidator_CorrectInitialization_NoExceptions()
{
Assert.DoesNotThrow(() => new NumberValidator(1, 0, true),
"При создании экземпляра класса не должно было возникнуть ошибок");
Copy link

@Inree Inree Nov 7, 2024

Choose a reason for hiding this comment

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

Если мы тут за fluent assertions, то есть func.Should().Throw(exception) и func.Should().NotThrow(exception)


[TestCase("00.00", 3, 2)]
[TestCase("+1.23", 3, 2)]
public void IsValidNumber_NotTheAppropriateLength_False(string value,
Copy link

Choose a reason for hiding this comment

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

Тут ещё по общим правилам нейминга должно быть типа "Should_ReturnFalse_WhenUnappropriateLength" или как-то так, но видимо вы не это проходили, так что можно не править

bool onlyPositive = true)
{
ClassicAssert.IsFalse(new NumberValidator(precision, scale, onlyPositive).IsValidNumber(value),
"Длина числа не соответствует шаблону");
Copy link

Choose a reason for hiding this comment

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

Ну и тут тоже new NumberValidator(precision, scale, onlyPositive).IsValidNumber(value).Should().Befalse();

Comment on lines 11 to 18
public void NumberValidator_IncorrectPrecision_Exception()
{
Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, true),
"precision должен быть положительным числом");
}

[Test]
public void NumberValidator_CorrectInitialization_NoExceptions()
Copy link

Choose a reason for hiding this comment

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

В этих тестах потеряны кейсы когда onlyPositive == false (в старом коде дублирование, ты можешь воткнуть TestCase(true) и TestCase(false))

И ещё если мы конструктор валидируем, то надо докинуть проверку scale: должен быть не меньше нуля и не больше precision

Copy link
Author

Choose a reason for hiding this comment

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

И ещё, если мы проверяем конструктор, то нужно добавить проверку масштаба: он должен быть не меньше нуля и не больше точности

у меня это проверяется в методе NumberValidator_IncorrectScale_Exception

Copy link

Choose a reason for hiding this comment

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

А, пон, не увидел


[TestCase("", 1)]
[TestCase(null, 1)]
public void IsValidNumber_NullOrEmpty_False(string value, int precision, int scale = 0, bool onlyPositive = true)
Copy link

Choose a reason for hiding this comment

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

Тут и в IsValidNumber_CorrectValue_True стоит onlyPositive, но в кейсах не указывается

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