Skip to content

5. MyNUnit - #6

Open
ArtemNikit1n wants to merge 3 commits into
mainfrom
05-my-nunit
Open

5. MyNUnit#6
ArtemNikit1n wants to merge 3 commits into
mainfrom
05-my-nunit

Conversation

@ArtemNikit1n

Copy link
Copy Markdown
Owner

No description provided.

Comment on lines +170 to +171
var assembly = Assembly.GetExecutingAssembly();
var assemblyPath = assembly.Location;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Это, наверное, тоже можно в SetUp вынести, во всех тестах оно есть.

[Before]
public void Before()
{
BeforeWasCalledCount++;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

По идее Before должен вызываться для каждого экземпляра тестового класса для каждого теста параллельно. А тут делается ++, а не Interlocked.Increment, для статического поля. Если тест проходит, то либо что-то не настолько параллельно, насколько надо, либо просто повезло.

public static void BeforeClass()
{
BeforeClassWasCalled = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут тоже незазорно использовать =>.

var beforeMethods = type.GetMethods()
.Where(m => m.GetCustomAttribute<BeforeAttribute>() != null && !m.IsStatic).ToList();
var afterMethods = type.GetMethods()
.Where(m => m.GetCustomAttribute<AfterAttribute>() != null && !m.IsStatic).ToList();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь бы больше проверок на правильность тестов и вспомогательных методов. Например, что тестовый метод не статический, ничего не возвращает и не принимает аргументов. Иначе упадёт при запуске теста, с невнятной ошибкой.

foreach (var testResult in testsInClass.Select(testInfo => new TestResult(
testInfo.TestClass.Name,
testInfo.TestMethod.Name,
TestStatus.Failed,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Он не совсем Failed. Сам тест даже не запускался, с ним, может быть, всё хорошо. Я бы добавил ещё один статус, "Errored" для обозначения тестов, которые не отработали из-за ошибки во вспомогательных методах.

var actualException = ex is TargetInvocationException tie ? tie.InnerException ?? ex : ex;
foreach (var result in results)
{
result.AddMessage($"\nWarning: AfterClass failed: {actualException.Message}");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Я бы такие тесты тоже как Failed помечал. Всё равно их надо чинить, и вдруг в AfterClass какая-то важная деинициализация, без которой результаты следующих прогонов будут невалидны (например, было бы глупо проверять успешность сборки программы, не удалив старые временные файлы).

object? testInstance = null;
if (!testInfo.TestMethod.IsStatic)
{
testInstance = Activator.CreateInstance(testInfo.TestClass);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут тоже может быть брошено исключение, это ведь вызовет конструктор, который может быть определён пользователем.

/// Gets the type of exception that is expected from the test.
/// </summary>
public Type? ExpectedException { get; } = expectedException;
} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут надо было record, а не класс, компилятор бы 4/5 этого файла сам сгенерил :)

/// Is applied to the methods of the class instance.
/// </summary>
[AttributeUsage(AttributeTargets.Method)]
public class AfterAttribute : Attribute;

Copy link
Copy Markdown

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.

3 participants