В порядке ли создание объектов с нулевыми параметрами в модульных тестах?

37

Я начал писать тестовые модули для моего текущего проекта. У меня действительно нет опыта с этим все же. Сначала я хочу полностью «получить его», поэтому в настоящее время я не использую ни свою платформу IoC, ни библиотеку-макет.

Мне было интересно, если что-то не так с предоставлением нулевых аргументов конструкторам объектов в модульных тестах. Позвольте мне привести пример кода:

public class CarRadio
{...}


public class Motor
{
    public void SetSpeed(float speed){...}
}


public class Car
{
    public Car(CarRadio carRadio, Motor motor){...}
}


public class SpeedLimit
{
    public bool IsViolatedBy(Car car){...}
}

Еще один пример автомобильного кода (ТМ), сводится только к тем частям, которые важны для вопроса. Я сейчас написал тест примерно так:

public class SpeedLimitTest
{
    public void TestSpeedLimit()
    {
        Motor motor = new Motor();
        motor.SetSpeed(10f);
        Car car = new Car(null, motor);

        SpeedLimit speedLimit = new SpeedLimit();
        Assert.IsTrue(speedLimit.IsViolatedBy(car));
    }
}

Тест работает нормально. SpeedLimitНужно Carс собой Motor, чтобы делать свое дело. Это совсем не интересно CarRadio, поэтому я предоставил для этого значение null.

Мне интересно, является ли объект, обеспечивающий правильную функциональность, не будучи полностью сконструированным, нарушением SRP или запахом кода. У меня есть это ноющее чувство, что оно есть, но speedLimit.IsViolatedBy(motor)и не правильно - ограничение скорости нарушается автомобилем, а не мотором. Может быть, мне просто нужна другая точка зрения для модульных тестов по сравнению с рабочим кодом, потому что все намерение состоит в том, чтобы протестировать только часть целого.

Является ли создание объектов с нулем в модульных тестах запахом кода?

Р. Шмитц
источник
24
Немного не по теме, но, Motorвероятно, не должно быть speedвообще. Он должен иметь throttleи вычислять на torqueоснове текущего rpmи throttle. Работа автомобиля состоит в том, чтобы использовать его, Transmissionчтобы интегрировать это в текущую скорость, и превратить это в rpmсигнал, чтобы сигнализировать Motor... Но я полагаю, вы все равно не были заинтересованы в реализме, не так ли?
Мастер
17
Запах кода в том, что ваш конструктор принимает ноль, не жалуясь в первую очередь. Если вы считаете, что это приемлемо (у вас могут быть на это причины): протестируйте!
Томми
4
Рассмотрим шаблон Null-Object . Это часто упрощает код, а также делает его NPE-безопасным.
Бакуриу
17
Поздравляем : вы проверили, что с nullрадио правильно рассчитывается ограничение скорости. Теперь вы можете создать тест для проверки ограничения скорости с помощью радио; на всякий случай поведение отличается ...
Мэтью М.
3
Не уверен насчет этого примера, но иногда тот факт, что вы хотите протестировать только половину класса, является подсказкой того, что что-то должно быть разбито
Оуэн

Ответы:

70

В случае приведенного выше примера разумно, что a Carможет существовать без a CarRadio. В этом случае я бы сказал, что не только допустимо CarRadioиногда принимать значение NULL , но я бы сказал, что это обязательно. Ваши тесты должны гарантировать, что реализация Carкласса является устойчивой и не генерирует исключения нулевого указателя, когда no CarRadioприсутствует.

Однако давайте возьмем другой пример - давайте рассмотрим SteeringWheel. Давайте предположим, что a Carдолжен иметь a SteeringWheel, но тест скорости на самом деле не заботится об этом. В этом случае я бы не пропустил ноль, так SteeringWheelкак это толкает Carкласс в места, где он не предназначен. В этом случае вам лучше создать некую разновидность DefaultSteeringWheel(для продолжения метафоры), заблокированную по прямой линии.

Пит
источник
44
И не забудьте написать модульный тест, чтобы проверить, что Carневозможно построить без SteeringWheel...
cmaster
13
Я мог бы что-то упустить, но если возможно и даже обычно создавать Carбез без CarRadio, не имеет ли смысла создавать конструктор, который не требует, чтобы его передавали, и вообще не беспокоился о явном нуле ?
вкладыш
16
Самостоятельно управляемые автомобили могут не иметь рулевого колеса. Просто говорю. Black
Блэкджек
1
@James_Parsons Я забыл добавить, что речь шла о классе, который не имел нулевых проверок, потому что он использовался только внутри и всегда получал ненулевые параметры. Другие методы Carбросили бы NRE с нулевым значением CarRadio, но соответствующий код для SpeedLimitне будет. В случае вопроса, который был опубликован сейчас, вы были бы правы, и я бы действительно сделал это.
Р. Шмитц
2
@mtraceur То, как все предполагали, что класс, допускающий конструирование с нулевым аргументом, означает ноль, является допустимой опцией, заставило меня понять, что, вероятно, там должны быть нулевые проверки. Фактическая проблема, которая у меня возникла, покрыта множеством хороших, интересных, хорошо написанных ответов, которые я не хочу разрушать и которые мне помогли. Также принимаю этот ответ сейчас, потому что я не люблю оставлять вещи незавершенными, и это наиболее одобрено (хотя все они были полезны).
Р. Шмитц
23

Является ли создание объектов с нулем в модульных тестах запахом кода?

Да. Обратите внимание на определение запаха кода в C2 : «CodeSmell - это намек на то, что что-то может быть не так, а не определенность».

Тест работает нормально. SpeedLimit нужен Автомобиль с Мотором, чтобы делать свое дело. Он совершенно не заинтересован в CarRadio, поэтому я предоставил для этого значение null.

Если вы пытаетесь продемонстрировать, что speedLimit.IsViolatedBy(car)это не зависит от CarRadioаргумента, переданного конструктору, то будущему сопровождающему, вероятно, будет понятнее сделать это ограничение явным, введя двойной тест, который не пройдёт тест, если он вызывается.

Если, как более вероятно, вы просто пытаетесь сохранить себе работу по созданию того, CarRadioчто, как вы знаете, не используется в этом случае, вы должны заметить, что

  • это нарушение инкапсуляции (вы позволяете CarRadioнеиспользованному факту просочиться в тест)
  • Вы обнаружили, по крайней мере, один случай, когда вы хотите создать Carбез указанияCarRadio

Я сильно подозреваю, что код пытается сказать вам, что вы хотите конструктор, который выглядит как

public Car(IMotor motor) {...}

и тогда этот конструктор может решить, что делать с тем, что нетCarRadio

public Car(IMotor motor) {
    this(null, motor);
}

или

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

или

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

и т.п.

Речь идет о передаче null чему-то другому во время тестирования SpeedLimit. Вы можете видеть, что тест называется «SpeedLimitTest», и единственный Assert проверяет метод SpeedLimit.

Это второй интересный запах - для того, чтобы протестировать SpeedLimit, вы должны построить вокруг автомобиля всю машину, хотя единственное, что, вероятно, беспокоит проверка SpeedLimit - это скорость .

Это может быть подсказка, которая SpeedLimit.isViolatedByдолжна принимать интерфейс роли, который реализует автомобиль , а не требовать всего автомобиля.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeedдействительно паршивое имя; надеюсь, ваш язык домена будет улучшен.

С этим интерфейсом ваш тест на SpeedLimitможет быть намного проще

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
VoiceOfUnreason
источник
В вашем последнем примере я предполагаю, что вам придется создать фиктивный класс, который реализует IHaveSpeedинтерфейс, поскольку (по крайней мере, в c #) вы не сможете создать экземпляр самого интерфейса.
Райан Сирл
1
Это верно - эффективное написание будет зависеть от того, какой аромат Blub вы используете для своих тестов.
VoiceOfUnreason
18

Является ли создание объектов с нулем в модульных тестах запахом кода?

нет

Не за что. Напротив, если NULLэто значение, которое доступно в качестве аргумента (некоторые языки могут иметь конструкции, которые запрещают передачу нулевого указателя), вы должны проверить это.

Другой вопрос, что происходит, когда вы проходите NULL. Но это именно то , что представляет собой модульный тест: убедиться, что определенный результат происходит, когда для каждого возможного ввода. И NULL это потенциальный вход, так что вы должны иметь определенный результат , и вы должны иметь испытание для этого.

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

В любом случае, да, вы должны написать тест для NULL. Это был бы запах кода, если бы вы его оставили. Это означало бы, что у вас есть непроверенная ветвь кода, которая, как вы только что предполагали, будет волшебным образом не содержать ошибок.


Обратите внимание, что я согласен с другими ответами, что ваш тестируемый код может быть улучшен. Тем не менее, если улучшенный код имеет параметры, которые являются указателями, передача NULLдаже для улучшенного кода является хорошим тестом. Я хотел бы, чтобы тест показал, что происходит, когда я прохожу NULLкак мотор. Это не запах кода, это противоположность запаху кода. Это тестирование.

nvoigt
источник
Похоже, вы пишете Carздесь о тестировании . Хотя я ничего не вижу, но считаю неправильное утверждение, вопрос касается тестирования SpeedLimit.
Р. Шмитц
@ R.Schmitz Не уверен, что ты имеешь в виду. Я процитировал вопрос, речь идет о переходе NULLк конструктору Car.
nvoigt
1
Речь идет о передаче нуля во время тестированияSpeedLimit . Вы можете видеть, что тест называется «SpeedLimitTest», и Assertпроверяется только метод SpeedLimit. Тестирование Car- например, «если ваш автомобиль должен быть сконструирован без радиоприемника, вы должны написать тест для этого» - будет происходить в другом тесте.
Р. Шмитц
7

Я лично не хотел бы вводить нули. Фактически, всякий раз, когда я внедряю зависимости в конструктор, первое, что я делаю, это вызывает ArgumentNullException, если эти инъекции равны нулю. Таким образом, я смогу безопасно использовать их в своем классе, не разнося десятки нулевых проверок. Вот очень распространенная модель. Обратите внимание на использование readonly, чтобы гарантировать, что эти зависимости, установленные в конструкторе, не могут быть установлены в null (или что-либо еще) впоследствии:

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

С учетом вышесказанного, я мог бы провести один или два модульных теста, в которых я вводил нули, просто чтобы убедиться, что мои нулевые проверки работают как положено.

Сказав это, это становится не проблема, когда вы делаете две вещи:

  1. Программа для интерфейсов вместо классов.
  2. Используйте фреймворк (например, Moq для C #), чтобы вводить фиктивные зависимости вместо нулей.

Так что для вашего примера у вас будет:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

Более подробную информацию об использовании Moq можно найти здесь .

Конечно, если вы хотите понять это без насмешек, вы все равно можете сделать это, если вы используете интерфейсы. Просто создайте фиктивные CarRadio и Motor, как показано ниже, и введите их вместо:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
Eternal21
источник
3
Это ответ, который я бы написал
Liath
1
Я бы даже сказал, что фиктивные объекты также должны выполнить свой контракт. Если IMotorбы был getSpeed()метод, DummyMotorон должен был бы вернуть измененную скорость и после успешного завершения setSpeed.
ComFreek
1

Это не просто хорошо, это хорошо известная техника взлома зависимостей для вставки устаревшего кода в тестовый комплект. (См. «Эффективная работа с устаревшим кодом» Майкла Фезерса.)

Это пахнет? Что ж...

Тест не пахнет. Тест делает свою работу. Он объявляет «этот объект может быть нулевым, а тестируемый код по-прежнему работает правильно».

Запах находится в реализации. Объявляя аргумент конструктора, вы заявляете: «Этому классу нужна эта вещь для правильного функционирования». Очевидно, это не соответствует действительности. Все, что не соответствует действительности, немного вонючее.

Резиновая утка
источник
1

Давайте сделаем шаг назад к первым принципам.

Модульный тест проверяет некоторые аспекты одного класса.

Однако будут конструкторы или методы, которые принимают другие классы в качестве параметров. То, как вы справляетесь с этим, будет варьироваться от случая к случаю, но установка должна быть минимальной, поскольку они касаются цели. Могут быть сценарии, в которых передача значения NULL совершенно допустима, например, автомобиль без радиоприемника.

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

Все идет нормально. Однако, что если NULL не является допустимым значением. Ну, вот вы попали в мрачный мир издевательств, заглушек, манекенов и подделок .

Если все это кажется слишком сложным, вы фактически уже используете пустышку, передавая NULL в конструкторе Car, поскольку это значение, которое потенциально не будет использоваться.

То, что должно стать ясным довольно быстро, это то, что вы потенциально можете нанести удар по своему коду с большой обработкой NULL. Если вы замените параметры класса интерфейсами, то вы также проложите путь для насмешек, DI и т. Д., Что делает их намного проще в обращении.

Робби Ди
источник
1
«Модульные тесты предназначены для тестирования кода одного класса». Вздох . Это не то, чем являются юнит-тесты, и следование этому руководству приведет вас к раздутым классам или контрпродуктивным, мешающим тестам.
Муравей P
3
К сожалению, восприятие «единицы» как эвфемизма для «класса» - очень распространенный способ мышления, но на самом деле все, что он делает, - это (а) поощряет тесты «мусора в мусоре», которые могут полностью подорвать достоверность всего теста комплекты и (b) обеспечивают границы, которые должны быть свободны для изменения, что может нанести ущерб производительности. Я хотел бы, чтобы больше людей слушали их боль и оспаривали это предубеждение.
Муравей Р
3
Нет такой путаницы. Тестовые единицы поведения, а не единицы кода. В концепции модульного теста нет ничего, кроме широко распространенного мнения о тестировании программного обеспечения, относящегося к культу грузов, что подразумевает, что тестовый модуль связан с отчетливо ООП-концепцией класса. И это тоже очень вредное заблуждение.
Муравей Р
1
Я не уверен точно, что вы имеете в виду - я спорю прямо противоположно тому, что вы намекаете. Заглушите / смоделируйте жесткие границы (если вам абсолютно необходимо) и протестируйте поведенческий блок. Через публичный API. Что это за API, зависит от того, что вы создаете, но занимаемая площадь должна быть минимальной. Добавление кода абстракции, полиморфизма или реструктуризации не должно приводить к сбою тестов - «единица на класс» противоречит этому и в первую очередь подрывает ценность тестов.
Муравей Р
1
RobbieDee - я говорю о прямо противоположном. Подумайте, что вы можете быть одним из тех, кто питает распространенные заблуждения, пересмотреть то, что вы считаете «единицей», и, возможно, немного глубже погрузиться в то, о чем на самом деле говорил Кент Бек, когда говорил о TDD. То, что большинство людей называют TDD сейчас, является чудовищем грузового культа. youtube.com/watch?v=EZ05e7EMOLM
Муравей P
1

Взглянув на ваш дизайн, я бы предположил, что Carон состоит из набора Features. Функция может быть CarRadio, или EntertainmentSystemкоторая может состоять из таких вещей, как CarRadio, DvdPlayerи т. Д.

Таким образом, как минимум, вы должны построить Carс набором Features. EntertainmentSystemи CarRadioбудут реализациями интерфейса (Java, C # и т. д.), public [I|i]nterface Featureи это будет экземпляр шаблона Composite.

Следовательно, вы всегда будете создавать Carс, Featuresа модульный тест никогда не будет создавать экземпляр Carбез него Features. Хотя вы можете подумать о BasicTestCarFeatureSetтом, чтобы использовать макет с минимальным набором функций, необходимых для вашего самого простого теста.

Просто некоторые мысли.

Джон

johnd27
источник