Skip to content

N unit#25

Open
Palezehvat wants to merge 6 commits intomasterfrom
NUnit
Open

N unit#25
Palezehvat wants to merge 6 commits intomasterfrom
NUnit

Conversation

@Palezehvat
Copy link
Owner

No description provided.

Copy link

@YuriUfimtsev YuriUfimtsev left a comment

Choose a reason for hiding this comment

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

Пока много предупреждений компилятора и мало тестов. Ну и структуру надо бы в порядок привести

Choose a reason for hiding this comment

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

Атрибуты надо по файлам разнести: один файл -- один класс. У каждого должен быть конструктор (даже пустой). И обязательно нужно их называть в виде ...Attribute. Здесь, например, TestAttribute

Choose a reason for hiding this comment

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

Почему здесь All, а не Method? Все эти шесть атрибутов же для методов предназначены

Choose a reason for hiding this comment

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

Если этот класс необходим родительскому классу и не нужен внешнему миру, то надо его сделать private-вложенным. Если же нужно иметь возможность обращаться к его свойствам/методам, то надо его в отдельный файл вынести

Choose a reason for hiding this comment

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

public => свойство => с большой буквы :)

@YuriUfimtsev
Copy link

Атрибуты теперь куда-то пропали


namespace MyNUnit;

public class ApplicationForTests

Choose a reason for hiding this comment

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

Надо комментарии


using MyNUnit.Atributes;

namespace MyNUnit;

Choose a reason for hiding this comment

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

namespace лучше первой строкой файла, сразу после шапки с лицензией


public class ApplicationForTests
{
public readonly List<ResultsTests> listOfResults = new();

Choose a reason for hiding this comment

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

public-всё в .NET именуется с заглавной. Но вообще, плохая идея делать мутабельный ссылочный тип readonly public-полем. Любой извне сможет взять и положить в список какой-то элемент — потому что readonly только ссылка, а не то, на что она указывает.

Parallel.ForEach(classes, StartTests);
}

private static MethodInfo[]? GetMethodsByAtributeAndClass(Type _class, Type atribute)

Choose a reason for hiding this comment

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

Attribute пишется с двумя t, прошу прощения за придирку :)

var methodsBefore = GetMethodsByAtributeAndClass(_class, typeof(BeforeAttribute));
var methodsAfter = GetMethodsByAtributeAndClass(_class, typeof(AfterAttribute));
var methods = _class.GetMethods();
foreach(var method in methods)

Choose a reason for hiding this comment

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

Suggested change
foreach(var method in methods)
foreach (var method in methods)

{
throw new NullReferenceException();
}
foreach(var method in methods)

Choose a reason for hiding this comment

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

Suggested change
foreach(var method in methods)
foreach (var method in methods)

}
}
}
catch(Exception ex)

Choose a reason for hiding this comment

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

Suggested change
catch(Exception ex)
catch (Exception ex)

var exceptionType = ex.GetType();

if (testAttribute != null && testAttribute.Expected != null && testAttribute.Expected.IsAssignableFrom(exceptionType) ||
(ex.InnerException != null && testAttribute != null && testAttribute.Expected != null && testAttribute.Expected.IsAssignableFrom(ex.InnerException.GetType())))

Choose a reason for hiding this comment

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

Сложные условия стоит выносить в отдельный метод

[AttributeUsage(AttributeTargets.Method)]
public class AfterAttribute : Attribute
{
public AfterAttribute() { }

Choose a reason for hiding this comment

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

Это не нужно, пустой конструктор сгенерируется сам, если в классе нет конструкторов

Comment on lines +9 to +13
[AfterAttribute]
public void Method()
{
;
}

Choose a reason for hiding this comment

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

Это вдвойне не нужно :)

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.

4 participants

Comments