Skip to content

Comments

MyNUnit#6

Open
Andrw-404 wants to merge 12 commits intomainfrom
MyNUnit
Open

MyNUnit#6
Andrw-404 wants to merge 12 commits intomainfrom
MyNUnit

Conversation

@Andrw-404
Copy link
Owner

No description provided.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

Choose a reason for hiding this comment

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

Подозреваю, что всё это не нужно

public void ThrowsExpectedException()
{
throw new ArgumentException();
}

Choose a reason for hiding this comment

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

Если это тестовый класс, это не повод не соблюдать стайлгайд :) Используйте =>, шапку с лицензией и всё такое.


public class LifecycleTest
{
public static List<string> Loger = new List<string>();

Choose a reason for hiding this comment

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

Он должен быть "Logger" по идее, и второй List<string> можно не писать.

public void ResetStat()
{
Loger.Clear();
}

Choose a reason for hiding this comment

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

Тут тем более стоит использовать =>

[Test]
public void BasicTests_ShouldIdentifySpecificity()
{
var result = this.runner!.RunTest(this.path!);

Choose a reason for hiding this comment

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

Проинициализировали бы runner и path какими-то значениями, чтобы сделать их ненуллабельными и тут ! не писать. ! тут явно не по делу.

try
{
instance = Activator.CreateInstance(testClass);
this.RunBeforeAndAfterMethods(testClass, instance!, typeof(BeforeAttribute));

Choose a reason for hiding this comment

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

Не используйте ! в "боевом" коде.

catch (TargetInvocationException)
{
throw;
}

Choose a reason for hiding this comment

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

Это кажется что абсолютно бессмысленная конструкция, поймали исключение и тут же перебросили его же

}
}

this.RunBeforeAndAfterMethods(testClass, instance!, typeof(AfterAttribute));

Choose a reason for hiding this comment

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

Тут тоже могут быть исключения и тест надо пометить как Errored — даже если он прошёл, всё равно чинить тесты надо.

this.RunSingleTest(testClass, testMethod, results, assemblyName);
}

this.RunStaticMethods(testClass, typeof(AfterClassAttribute), out _);

Choose a reason for hiding this comment

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

Тут тоже

return results.ToList();
}

private bool RunStaticMethods(Type type, Type attributeType, out string? error)

Choose a reason for hiding this comment

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

out-параметры вроде как не нужны в современном C#, возвращайте лучше кортеж. Правильнее всего было бы вообще размеченное объединение Success | Error of string, но C# такого не умеет, а делать аналог вручную слишком громоздко.

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