Pull to refresh

Comments 18

А зачем так сложно? Проверить исходники по критерию
Должен быть NotNull у всех параметров которые:
public method
не @Nullable
не примитив

Можно гораздо проще на этапе прекоммит хуков или в любом другом месте вашей CI. И не надо никаких ничего не тестирующих тестов.
а как Ваша хука внтутри работает не думали? а самому написать не хотелось? CI это хорошо, но тот же SOnar требует явного тестирования аннотаций, а не наличия аннотаций. Если бы вы писали JVM мне страшно подумать как бы осуществлялась верификация в байткод, например

Я в курсе как хуки внутри работают.


Тестировать библиотечные функции своими тестами это чистый антипаттерн. Библиотеки должны тестироваться тестами библиотек.


Когда пишешь jit тестируешь тоже jit. Когда пишешь бизнес логику тестируешь тоже бизнес логику. Все нижележащие уровни принимаются правильно работающими и не тестируются.

Тестировать библиотечные функции своими тестами это чистый антипаттерн

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

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

Когда кто-нибудь из моих младших коллег выкатывает нечто подобное, я обычно пишу в ревью что-то типа «если решение задачи получается настолько неожиданно громоздким и хрупким, возможно, стоило задуматься над правильностью её формулировки, прежде чем начинать писать код».
Не, это вы зря. Я не про решение — возможно оно реально слишком громоздкое. Я про формулировку задачи — к сожалению, мы имеем то, что имеем. Если мы хотим в Java повесить на методы какую-то стороннюю логику, то аннотации — один из реально применимых способов. А вот что там дальше происходит в коде в момент выполнения — зачастую понять не так просто.

Представьте, что у вас вместо @Nullable стоит @Transactional? Как вы предлагаете тестировать поведение метода с этой аннотацией? Как вы разделите поведение самого метода, и той логики, которую на него навесил условный ломбок (или условный спринг)? Задача вполне реальная, и я думаю надо автору сказать спасибо за то, что на ней было акцентировано внимание.
то то и оно, статья не руководство к действию, а лишь повод и средство более детально понять как работают аннотации, как рефлексия и прокси на объекты.
Автору выше, который с юными коллегами: а Вы Spring тоже брезгуете юзать? или предпочитчаете юзать не думая, что внутри него есть рефлексия или Вам приятно думать что аннотация работает сама по себе?
Это небольшой тестовый фреймворк, даже если конкретно эта задача Вам кажется фейковой, все то что в ней использовано — используется в джавовых фреймворках.
Это «громозлкое решение» — это 180 страниц кода и один класс, странное у вас представление о громоздкости. Попробуйте сломать, это «хрупкое решение». И это тест, утилька для того чтобы убрать повторение кода по вашим тестам. Да, мне не кажется что это самые полезные тесты, на аннотацию @NonNull, но мой тим лид требует тестить, дальше что? Странно, что при всем Вашем опыте, Вы не знаете, что проекты разные бывают и требования на них те же, хотя судя по пафосу, предположу что вы сидите 10 лет на одном проекте и брызжете пафосом на более молодых колег.
P.S наверняка со своей принципиальностью в общем потоке просмотрели как не ок, так и годные идеи.
P.S.S Я бы вас с удовольствием отревъювил. Со всеми выкладками на доки и мировые best practices.

Я мог бы ответить конструктивно, но вы уже перешли на личности.


Я знаю, как работает рефлексия, писал и на спринге, и на EE (от разных вендоров) и даже собирал собственный контейнер из запчастей. Дважды, в разных окружениях. Это не говоря о плагинах для мавена и грэдла для нужд автоматизации разработки моих проектов. Таки 20 лет на джаве пишу с периодическими перерывами на другие платформы, потому меня и берут в команду как эксперта или играющего тренера.


Давайте лучше не будем меряться толщиной mojo.


Касательно «пафоса» — я искренне благодарен вам за красивый антипример антипаттерна. Спасибо.

C @Transactional проблем нет, коли он рантаймовый. Клин клином, аспект поверх аспекта, или можно в контейнер вклиниться, благо, большинство позволяют.

Автор же зачем-то тестирует наличие аннотации, у которой Retention=SOURCE. На этапе выполнения. Если уж на самом деле такое настолько нужно, то это зона ответственности компилятора. Следовательно, выбран не тот инструмент на неправильной фазе жизненного цикла.
>это зона ответственности компилятора.
Ну, мне кажется — не обязательно. Например какого-то AST трансформера.

Опять же — я сразу допустил, что инструмент не тот. Что вы можете предложить лучше для тестирования аннотаций? Ну так чтобы практически проверить?

>аспект поверх аспекта, или можно в контейнер вклиниться, благо, большинство позволяют.
Ну и чем это будет отличаться от описанного? :) Тоже ведь будет «магия» вполне возможно.
А ко всем остальным минусерам, без аргументов, поражает ваше стадное чувство.
Как только я опубликовал статью, в ту же секунду кинул ее в один из своих чатов в телеграм, в ту же секунду появился первый просмотр и первый минус, то есть даже прочитать название до конца было нереально. Ну а дальше стадное чувство сработало. Спасибо sshikov, что остановили это бред.
Я может быть чего-то не понял, но. Получается вы взяли ломбок, взяли его аннотацию, и написали свой минифреймворк, который проверяет, что ломбок корректно обрабатывает собственную аннотацию во всех случаях, когда вы ее используете. Это как-то… избыточно. Логика подсказывает, что либо ломбок работает корректно — везде! — либо не корректно, но опять же — везде. То есть достаточно проверить только один раз, при чем можно специальный тестовый кейс. Или я чего-то не понял?
У меня ощущение что эти тесты, сами нуждаются в тестировании. Ну в смысле по моему тест должен выглядеть проще чем тестируемый код.
Обратите внимание на создание Enum(вишенка на торте), вообщем нельзя просто так взять и создать Enum.


Не совсем понятен код:
    if (parameter.getType().isEnum()) {
        methodParam[index] = Enum.valueOf(
            (Class<Enum>) (parameter.getType()),
            parameter.getType().getEnumConstants()[0].toString()
        );
      }

В чем вишенка и почему нельзя упростить:
    if (parameter.getType().isEnum()) {
        methodParam[index] = parameter.getType().getEnumConstants()[0];
    }


С enum действительно все не просто, такой код заработает в TestUtil:

    public enum ExtEnum {
    
    A("1") {}, // фигурные скобки заданы умышлено
    B("2") {}; // фигурные скобки заданы умышлено
    
    private final String text;
    
    
    ExtEnum(String text) {
        this.text = text;
    }
    
    String getTextWith(String title) {
        return title + ":" + text;
    }    
}

Статья навеяла несколько вопросов о работе с null:


  1. Можно ли прикрутить к @NonNull какие-нибудь статические проверки, что null туда реально не попадает? Что делать (и как вообще это узнать), когда аннотация поставлена неправильно?


  2. Стоит ли NPE в рантайме, которую генерирует Lombok, таких усилий по разметке аннотациями всех полей, тестированию этого всего? Будет ли сильно хуже, если все эти аннотации просто выкинуть?


  3. Почему нельзя использовать Optional? Оно хотя бы гарантирует корректную обработку nullable-значений на уровне компилятора, их уже не перепутаешь с non-null (хотя в обратную сторону — легко).


Моё мнение:
  1. Насколько я понимаю IDEA например может делать что-то такое. Но в чистой Java довольно сложно понять по переменной или по полю что там точно-точно не может быть null, без явных проверок в коде.
  2. Разметка аннотациями еще и для тех кто будет читать и использовать этот код, во многих методах непонятно можно ли передавать параметр по умолчанию null без чтения документации. Зачем тестировать аннотацию и кодогенерацию я не знаю.
  3. С Optional код часто выглядит хуже (субъективное мнение), ну и не спасёт от тех кто передаст null вместо Optional.

PS: Если проблема с null для вас актуальна, то я бы посоветовал попробовать Kotlin.

Попробую возразить по поводу Optional.


Возможность передачи null вместо Optional это конечно беда, от этого придется защищаться статическими проверками (вроде как Findbugs что-то умеет http://findbugs.sourceforge.net/bugDescriptions.html#NP_OPTIONAL_RETURN_NULL). Плюс видимо придется защищаться от Optional.get().


Код, конечно может выглядеть более громоздко, чем аннотация, но если учесть, что он реально защищает от NPE, а аннотации — нет, может оно того и стоит.

Отвечая на свой же первый вопрос, нагуглилась вот такая штука: https://checkerframework.org/manual/#nullness-checker. Есть ли у кого-нибудь опыт использования на практике? По идее, статическая проверка должна быть лучше, чем Lombok...


UPD: есть еще набор проверок в Findbugs: http://findbugs.sourceforge.net/bugDescriptions.html#NP_ALWAYS_NULL

Sign up to leave a comment.

Articles