Привет

Решил вот поучиться писать юнит-тесты. Придумал такой кусок кода в качестве "дано":

class NotepadService {
...
  public NoteDTO createNote(Long userId, String title, String text) throws NoSuchUserException {
    // 1
    User user = userRepository.findOne(userId);
    if(user == null) {
      throw new NoSuchUserException();
    }
    //

    // 2
    Note note = new Note();
    note.setOwner(user);
    note.setTitle(title);
    note.setText(text);
    note.setCreateAt(new Date());
    note.setModifiedAt(null);
    //
        
    note = noteRepository.save(note);

    user = userRepository.findOne(user.getId());

    // 3
    user.setLastActivityAt(new Date());
    //

    userRepository.save(user);
    
    // 4    
    return new NoteDTO(
        note.getId(),
        note.getOwner().getId(),
        note.getCreatedAt(),
        note.getModifiedAt(),
        note.getTitle(),
        note.getText());
    //
  }
...
}

и в качестве задачи — покрыть его функциональность тестами. Не интеграционными. Чтоб без базы.

Решение в лоб, которое приходит в голову:
1. Из блока №1 сделать класс вроде UserByUserIdResolver, тестировать его отдельно от всего.
2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
4. Из блока №4 сделать класс NoteToNoteDTOMapper, ну и тоже отдельно.

Из плюсов: все эти куски можно будет тестировать совершенно независимо, т.к. реально они отвечают за разные аспекты поведения. Код самого метода станет значительно короче и читабельнее.
Из минусов: это реально дохрена каких-то шумоподобных классов по 3 строчки. UserByUserIdResolver... что это за нафиг вообще? (в смысле — я недоволен смысловой нагрузкой абстракции, ощущается как TwoPlusTwoCalculator)

Подскажите пожалуйста как такое делать правильно ("ваш вариант") и на что обращать внимание?

Заранее спасибо!

Здравствуйте, andyag, Вы писали:

a> Решение в лоб, которое приходит в голову:
a> 1. Из блока №1 сделать класс вроде UserByUserIdResolver, тестировать его отдельно от всего.
Я бы сделал рядом с методом findOne метод getOne(long id) throws NoSuchUserException, ибо наверняка именно этот метод будет использоваться чаще всего. Кстати, имя лучше findById/getById.

a> 2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
Вряд ли этот метод будет использоваться из многих мест с ровно этим набором параметров, так что я бы оставил как есть.

a> 3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
в userRepository сделать метод touch(User user) (юниксойдам будет понятно) или updateActivity.

a> 4. Из блока №4 сделать класс NoteToNoteDTOMapper, ну и тоже отдельно.
Это да, ибо явно будет использоваться в нескольких местах.

a> Из плюсов: все эти куски можно будет тестировать совершенно независимо, т.к. реально они отвечают за разные аспекты поведения. Код самого метода станет значительно короче и читабельнее.
a> Из минусов: это реально дохрена каких-то шумоподобных классов по 3 строчки.
Если 3 строчки будут использоваться из как минимум 2 мест, то уже вполне оправданно.

a> UserByUserIdResolver... что это за нафиг вообще? (в смысле — я недоволен смысловой нагрузкой абстракции, ощущается как TwoPlusTwoCalculator)
Ага, нафиг.

a> Подскажите пожалуйста как такое делать правильно ("ваш вариант") и на что обращать внимание?
Не понял, каким боком вопрос с тестированием связан?

ЗЫЖ Почему repository? Обычно dao вроде как называют такое.
ЗЗЫЖ Дбля Аббревиатур лучше тоже camel-case использовать. Во-первых удобнее набирать, во-вторых красивее, особенно если несколько аббревиатур в слове. Например есть сущность VAT (Value Added Tax) и сравни — что проще читать — VATDTO или VatDto?

Здравствуйте, ., Вы писали:

.>Здравствуйте, andyag, Вы писали:

a>> Решение в лоб, которое приходит в голову:
a>> 1. Из блока №1 сделать класс вроде UserByUserIdResolver, тестировать его отдельно от всего.
.>Я бы сделал рядом с методом findOne метод getOne(long id) throws NoSuchUserException, ибо наверняка именно этот метод будет использоваться чаще всего. Кстати, имя лучше findById/getById.
Смысл "рефакторинга" — получить класс, объект готорого отвечает за поиск пользователя по id и кидает исключение, если юзера нет. Этот класс легко будет протестировать — всего 2 кейза его полностью покроют.

a>> 2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
.>Вряд ли этот метод будет использоваться из многих мест с ровно этим набором параметров, так что я бы оставил как есть.
Здесь опять же ради тестирования — с одной стороны назначение этого класса легко объяснить, т.к. у него всего одна ответственность, с другой — его так же легко будет тестировать.

a>> 3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
.>в userRepository сделать метод touch(User user) (юниксойдам будет понятно) или updateActivity.
По какой причине этот метод должен находиться в репозитории? http://martinfowler.com/eaaCatalog/repository.html — пишет "Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects". Стоит ли обновление last activity воспринимать как "скорее data access, нежели чем бизнес-логика"?

a>> 4. Из блока №4 сделать класс NoteToNoteDTOMapper, ну и тоже отдельно.
.>Это да, ибо явно будет использоваться в нескольких местах.
Согласен

a>> Из плюсов: все эти куски можно будет тестировать совершенно независимо, т.к. реально они отвечают за разные аспекты поведения. Код самого метода станет значительно короче и читабельнее.
a>> Из минусов: это реально дохрена каких-то шумоподобных классов по 3 строчки.
.>Если 3 строчки будут использоваться из как минимум 2 мест, то уже вполне оправданно.
Да вообще и одного места достаточно, вопрос в том, что это за 3 строчки.

a>> UserByUserIdResolver... что это за нафиг вообще? (в смысле — я недоволен смысловой нагрузкой абстракции, ощущается как TwoPlusTwoCalculator)
.>Ага, нафиг.

a>> Подскажите пожалуйста как такое делать правильно ("ваш вариант") и на что обращать внимание?
.>Не понял, каким боком вопрос с тестированием связан?
Как отрефакторить метод, чтобы функциональность, которая в нём находилась до рефакторинга, можно было бы протестировать не боясь комбинаторного взрыва (при росте числа аргументов) и без слишком сильной привязки к его реализации. И ещё чтобы это были тру функциональные тесты, а не интеграционные.

.>ЗЫЖ Почему repository? Обычно dao вроде как называют такое.
Вообще DAO — это "сохранять в базу, читать из базы" (save, delete), а repository — это "некая коллекция объектов" (add, remove), которая опционально может маппиться на базу. Как всегда, программисты всё понапутали и в Spring Data Neo4j репозиторием назвали то, что стоило бы назвать DAO.

.>ЗЗЫЖ Дбля Аббревиатур лучше тоже camel-case использовать. Во-первых удобнее набирать, во-вторых красивее, особенно если несколько аббревиатур в слове. Например есть сущность VAT (Value Added Tax) и сравни — что проще читать — VATDTO или VatDto?
Я бы написал VatDTO, у меня DTO это не "часть названия", а скорее суффикс-маркер.

Здравствуйте, andyag, Вы писали:

a> .>Я бы сделал рядом с методом findOne метод getOne(long id) throws NoSuchUserException, ибо наверняка именно этот метод будет использоваться чаще всего. Кстати, имя лучше findById/getById.
a> Смысл "рефакторинга" — получить класс, объект готорого отвечает за поиск пользователя по id и кидает исключение, если юзера нет. Этот класс легко будет протестировать — всего 2 кейза его полностью покроют.
Но почему обязательно класс? Пусть будет метод.

a> a>> 2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
a> .>Вряд ли этот метод будет использоваться из многих мест с ровно этим набором параметров, так что я бы оставил как есть.
a> Здесь опять же ради тестирования — с одной стороны назначение этого класса легко объяснить, т.к. у него всего одна ответственность, с другой — его так же легко будет тестировать.
createNote вполне точно говорит что там делается. А вот объяснять отличие create от make — мне кажется не так легко.

a> a>> 3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
a> .>в userRepository сделать метод touch(User user) (юниксойдам будет понятно) или updateActivity.
a> По какой причине этот метод должен находиться в репозитории? http://martinfowler.com/eaaCatalog/repository.html — пишет "Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects". Стоит ли обновление last activity воспринимать как "скорее data access, нежели чем бизнес-логика"?
Да, согласен, этот метод в бизнес-логике должен быть. Но явно должен быть как отдельный метод.

a> Да вообще и одного места достаточно, вопрос в том, что это за 3 строчки.
Сделай приватный метод с удобным названием, надо будет сделаешь публичным, напишешь тест. А так ты слишком много имплементации будешь делать публичной.

a> a>> Подскажите пожалуйста как такое делать правильно ("ваш вариант") и на что обращать внимание?
a> .>Не понял, каким боком вопрос с тестированием связан?
a> Как отрефакторить метод, чтобы функциональность, которая в нём находилась до рефакторинга, можно было бы протестировать не боясь комбинаторного взрыва (при росте числа аргументов) и без слишком сильной привязки к его реализации. И ещё чтобы это были тру функциональные тесты, а не интеграционные.
Не знаю, в приведённом коде функциональности немного. 2-3 тестами покрывается.

Кстати, непонятно зачем там userRepository.findOne вызывается дважды.

a> .>ЗЫЖ Почему repository? Обычно dao вроде как называют такое.
a> Вообще DAO — это "сохранять в базу, читать из базы" (save, delete), а repository — это "некая коллекция объектов" (add, remove), которая опционально может маппиться на базу. Как всегда, программисты всё понапутали и в Spring Data Neo4j репозиторием назвали то, что стоило бы назвать DAO.
Не обязательно в базу, а в хранилище. А что ещё репозиторий может делать кроме как куда-то персистить? Но в общем я с DDD дела не имел, так что тут не советчик.

a> .>ЗЗЫЖ Дбля Аббревиатур лучше тоже camel-case использовать. Во-первых удобнее набирать, во-вторых красивее, особенно если несколько аббревиатур в слове. Например есть сущность VAT (Value Added Tax) и сравни — что проще читать — VATDTO или VatDto?
a> Я бы написал VatDTO, у меня DTO это не "часть названия", а скорее суффикс-маркер.
А чем суффикс-маркер от не-маркера отличается? Почему тогда не NotepadSERVICE? DtoXmlUtils это уже префикс-маркер? В общем какие-то нечёткие правила, и непонятно что даёт, путаница на ровном месте.

Кстати, как вариант createNote может брать на вход NoteDto — если при создании придётся ещё передавать несколько других данных (комментарий какой-нибудь, важность и т.п.) — будет гораздо удобнее передать в метод один объект вместо 10 параметров.

Здравствуйте, ., Вы писали:

.>Здравствуйте, andyag, Вы писали:

a>> .>Я бы сделал рядом с методом findOne метод getOne(long id) throws NoSuchUserException, ибо наверняка именно этот метод будет использоваться чаще всего. Кстати, имя лучше findById/getById.
a>> Смысл "рефакторинга" — получить класс, объект готорого отвечает за поиск пользователя по id и кидает исключение, если юзера нет. Этот класс легко будет протестировать — всего 2 кейза его полностью покроют.
.>Но почему обязательно класс? Пусть будет метод.
Очень разумное обвинение Единственная формальная причина — в Java объекты можно инжектить, методы нельзя. Т.е. если для какого-то теста NotepadService мне понадобится специфическая "находилка юзеров по id" (например — для любого id ничего не находит), я смогу сконструировать такой объект NotepadService, который не сможет найти ни одного пользователя. Тут я совершенно честно соглашусь, что высасываю из пальца и пока такой сценарий не актуален.

a>> a>> 2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
a>> .>Вряд ли этот метод будет использоваться из многих мест с ровно этим набором параметров, так что я бы оставил как есть.
a>> Здесь опять же ради тестирования — с одной стороны назначение этого класса легко объяснить, т.к. у него всего одна ответственность, с другой — его так же легко будет тестировать.
.>createNote вполне точно говорит что там делается. А вот объяснять отличие create от make — мне кажется не так легко.
Create это "создать" (пафосно), make это "изготовить, произвести" (не пафосно, на то оно и внутри фабрики). Я здесь не очень понял — эти методы находятся в разных классах, вроде классы довольно полно описывают контекст — неужели правда может смутить?

a>> a>> 3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
a>> .>в userRepository сделать метод touch(User user) (юниксойдам будет понятно) или updateActivity.
a>> По какой причине этот метод должен находиться в репозитории? http://martinfowler.com/eaaCatalog/repository.html — пишет "Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects". Стоит ли обновление last activity воспринимать как "скорее data access, нежели чем бизнес-логика"?
.>Да, согласен, этот метод в бизнес-логике должен быть. Но явно должен быть как отдельный метод.
Здесь прослеживается сильная аналогия с findById/getById/findOne/getOne, про которые написано выше. Переименовать в updateActivity() или в touch() вполне можно. Вопрос в том, где этот метод должен находиться, чтобы можно было его во-первых протестировать, а во-вторых "заменить" на другой, если какому-то тесту вдруг это понадобится. Возможно снова высасываю из пальца.

a>> Да вообще и одного места достаточно, вопрос в том, что это за 3 строчки.
.>Сделай приватный метод с удобным названием, надо будет сделаешь публичным, напишешь тест. А так ты слишком много имплементации будешь делать публичной.
И снова хороший аргумент. А какими критериями стоит руководствоваться при вынесении/невынесении логики в отдельные классы? Вроде как в теории пишут, что классы должны быть маленькими, методы должны быть маленькими. С таким подходом как я описываю, вроде самое оно и получается. Но тут же возникает ощущение кашеобразности.

a>> a>> Подскажите пожалуйста как такое делать правильно ("ваш вариант") и на что обращать внимание?
a>> .>Не понял, каким боком вопрос с тестированием связан?
a>> Как отрефакторить метод, чтобы функциональность, которая в нём находилась до рефакторинга, можно было бы протестировать не боясь комбинаторного взрыва (при росте числа аргументов) и без слишком сильной привязки к его реализации. И ещё чтобы это были тру функциональные тесты, а не интеграционные.
.>Не знаю, в приведённом коде функциональности немного. 2-3 тестами покрывается.
Я полностью соглашусь, здесь логики почти нет, но вместе с этим не понятно как добиться ортогональности в тестах не разрезая всё на совсем маленькие куски. Например завтра возникнут задачи:
1. Сделать валидацию для title и text.
2. Ограничение "1 новая заметка в 10 секунд" для каждого пользователя.
3. Ограничение "не более 100 заметок в день" для каждого пользователя.
4. Ограничение "не более 1000 заметок в месяц" для каждого пользователя.

Послезавтра:
1. Дневные и месячные ограничения должны зависеть от типа подписки пользователя. У нас теперь есть 3 типа подписки!
2. Whatever else

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

.>Кстати, непонятно зачем там userRepository.findOne вызывается дважды.

Note и User — это POJO, а не прокси. Если второй раз не делать findOne(), связь между новым Note и User затрётся.

a>> .>ЗЫЖ Почему repository? Обычно dao вроде как называют такое.
a>> Вообще DAO — это "сохранять в базу, читать из базы" (save, delete), а repository — это "некая коллекция объектов" (add, remove), которая опционально может маппиться на базу. Как всегда, программисты всё понапутали и в Spring Data Neo4j репозиторием назвали то, что стоило бы назвать DAO.
.>Не обязательно в базу, а в хранилище. А что ещё репозиторий может делать кроме как куда-то персистить? Но в общем я с DDD дела не имел, так что тут не советчик.
Например, на работе сейчас репозиторий ничего не персистит, а живёт себе спокойно в памяти и синхронизуется с веб-сервисом.

a>> .>ЗЗЫЖ Дбля Аббревиатур лучше тоже camel-case использовать. Во-первых удобнее набирать, во-вторых красивее, особенно если несколько аббревиатур в слове. Например есть сущность VAT (Value Added Tax) и сравни — что проще читать — VATDTO или VatDto?
a>> Я бы написал VatDTO, у меня DTO это не "часть названия", а скорее суффикс-маркер.
.>А чем суффикс-маркер от не-маркера отличается? Почему тогда не NotepadSERVICE? DtoXmlUtils это уже префикс-маркер? В общем какие-то нечёткие правила, и непонятно что даёт, путаница на ровном месте.
Не стану спорить — стоит об этом задуматься.

.>Кстати, как вариант createNote может брать на вход NoteDto — если при создании придётся ещё передавать несколько других данных (комментарий какой-нибудь, важность и т.п.) — будет гораздо удобнее передать в метод один объект вместо 10 параметров.
Уже совсем оффтопик До 4 аргументов приемлемо, больше 4 пора делать CreateNoteRequest

Я тогда немного подкорректирую общий вопрос. Не думая о тестах я пишу код примерно так:

Пишу код. Вижу, что какой-то кусок слишком конкретен/детален для данного места — выношу в метод того же класса. Написал то, что хотел написать. Смотрю на класс, оцениваю ответственности — вижу пару методов, которые хотя используются только здесь, но с общим предназначением класса слабо вяжутся. Выношу в отдельный класс, делаю зависимость. Выглядит как ООП, классы получаются вполне вменяемыми, разобраться как это всё работает вполне можно. Но когда я пытаюсь покрыть написанное тестами, некоторые классы покрываются хорошо, а некоторые — хреново.

И вот вопрос — какими ценностями стоит руководствоваться, если хочется чтобы ВСЁ легко было протестировать. Отключив мозг я понимаю, что если порезать логику достаточно мелко, тестировать будет легко. Включив мозг я понимаю, что порезать логику достаточно мелко — значит утонуть в болоте из этой логики, потому что навскидку не так просто будет сказать "что же вообще делает эта программа и как она это делает", т.к. одна банальная операция создания новой заметки будет тянуть за собой в простейшем случае 5 объектов, а при добавлении новых требований, которые я описал выше их станет не менее 10. При этом тестироваться всё будет замечательно, но при это будет 10 классов. Для "почти" одного метода сервиса. Нет, они конечно будут реюзаться во всяких update/delete, но будет ли этот результат в итоге вменяемым?

Здравствуйте, andyag, Вы писали:

a> Очень разумное обвинение Единственная формальная причина — в Java объекты можно инжектить, методы нельзя. Т.е. если для какого-то теста NotepadService мне понадобится специфическая "находилка юзеров по id" (например — для любого id ничего не находит), я смогу сконструировать такой объект NotepadService, который не сможет найти ни одного пользователя. Тут я совершенно честно соглашусь, что высасываю из пальца и пока такой сценарий не актуален.
Эээ... а jmock/mockito используется?
А если продолжать в таком духе у тебя у всех классов будет по одному методу.

a> Create это "создать" (пафосно), make это "изготовить, произвести" (не пафосно, на то оно и внутри фабрики). Я здесь не очень понял — эти методы находятся в разных классах, вроде классы довольно полно описывают контекст — неужели правда может смутить?
Непонятно в чём соль разделение создания от изготовления.

a> .>Да, согласен, этот метод в бизнес-логике должен быть. Но явно должен быть как отдельный метод.
a> Здесь прослеживается сильная аналогия с findById/getById/findOne/getOne, про которые написано выше. Переименовать в updateActivity() или в touch() вполне можно. Вопрос в том, где этот метод должен находиться, чтобы можно было его во-первых протестировать, а во-вторых "заменить" на другой, если какому-то тесту вдруг это понадобится. Возможно снова высасываю из пальца.
Где-то среди сервисов юзера.

a> И снова хороший аргумент. А какими критериями стоит руководствоваться при вынесении/невынесении логики в отдельные классы? Вроде как в теории пишут, что классы должны быть маленькими, методы должны быть маленькими. С таким подходом как я описываю, вроде самое оно и получается. Но тут же возникает ощущение кашеобразности.
Выносить только если вдруг надо — переиспользование, нарушение SRP, слишком большой метод/класс.
Вообще, вынести — очень просто — есть куча рефакторингов (extract method, extract method object, extract class и т.п.). А вот делать обширный публичный интерфейс связывает развитие проекта, т.к. публичный интерфейс получается не продуманным, а от балды, и в итоге лезет спагетти.

a> .>Не знаю, в приведённом коде функциональности немного. 2-3 тестами покрывается.

a> Я полностью соглашусь, здесь логики почти нет, но вместе с этим не понятно как добиться ортогональности в тестах не разрезая всё на совсем маленькие куски. Например завтра возникнут задачи:
a> 1. Сделать валидацию для title и text.
Валидации должно быть две — в GUI, чтобы показывать красивые сообщения об ошибках и в сервисном слое, чтобы не поломать субд (кстати эта валидация уже частично есть в виде db constraints) и безопасность (чтобы нельзя было создать note от имени другого пользователя, например). Как видно, это не появится в твоём методе, а будет где-то в gui и в слое authn/authz.

a> 2. Ограничение "1 новая заметка в 10 секунд" для каждого пользователя.
a> 3. Ограничение "не более 100 заметок в день" для каждого пользователя.
a> 4. Ограничение "не более 1000 заметок в месяц" для каждого пользователя.
Сделаешь новый ThrottleService, добавишь от него зависимость в NotepadService, в существующих юнит-тестах появится один тупой мок, ничего не ограничивающий. И добавишь ещё один юнит тест с другим моком, который всегда ограничивает для проверки того, что createNote кидает исключение (или ещё как обламывается) при попытке превысить лимит.

a> Послезавтра:
a> 1. Дневные и месячные ограничения должны зависеть от типа подписки пользователя. У нас теперь есть 3 типа подписки!
a> 2. Whatever else
Open/Closed principle...

a> Сценарий абсолютно правдоподобный (хотя бы потому, что я намерен всё это проделать в качестве лабораторной). И тут, внимание вопрос, насколько выгодно окажется порезать логику максимально мелко прямо сейчас — вплоть до "1 действие — 1 класс"?
Не знаю. Порезать можно и потом, имхо.

a> .>Кстати, непонятно зачем там userRepository.findOne вызывается дважды.

a> Note и User — это POJO, а не прокси. Если второй раз не делать findOne(), связь между новым Note и User затрётся.
Почему? Ты же в user ничего не меняешь, кроме даты. Короче всё равно непонятно, но может я не знаю что-то о твоём persistence layer.

a> .>Не обязательно в базу, а в хранилище. А что ещё репозиторий может делать кроме как куда-то персистить? Но в общем я с DDD дела не имел, так что тут не советчик.
a> Например, на работе сейчас репозиторий ничего не персистит, а живёт себе спокойно в памяти и синхронизуется с веб-сервисом.
Персистит в веб-сервис же... Не чувствую разницы.

a> Уже совсем оффтопик До 4 аргументов приемлемо, больше 4 пора делать CreateNoteRequest
Согласен.

a> Я тогда немного подкорректирую общий вопрос. Не думая о тестах я пишу код примерно так:

a> Пишу код. Вижу, что какой-то кусок слишком конкретен/детален для данного места — выношу в метод того же класса. Написал то, что хотел написать. Смотрю на класс, оцениваю ответственности — вижу пару методов, которые хотя используются только здесь, но с общим предназначением класса слабо вяжутся. Выношу в отдельный класс, делаю зависимость. Выглядит как ООП, классы получаются вполне вменяемыми, разобраться как это всё работает вполне можно. Но когда я пытаюсь покрыть написанное тестами, некоторые классы покрываются хорошо, а некоторые — хреново.
TDD может попробовать? Обычно если код легко тестировать, то его легко понять. Но не наоборот.

a> И вот вопрос — какими ценностями стоит руководствоваться, если хочется чтобы ВСЁ легко было протестировать. Отключив мозг я понимаю, что если порезать логику достаточно мелко, тестировать будет легко. Включив мозг я понимаю, что порезать логику достаточно мелко — значит утонуть в болоте из этой логики, потому что навскидку не так просто будет сказать "что же вообще делает эта программа и как она это делает", т.к. одна банальная операция создания новой заметки будет тянуть за собой в простейшем случае 5 объектов, а при добавлении новых требований, которые я описал выше их станет не менее 10. При этом тестироваться всё будет замечательно, но при это будет 10 классов. Для "почти" одного метода сервиса. Нет, они конечно будут реюзаться во всяких update/delete, но будет ли этот результат в итоге вменяемым?
Мне кажется, что простота тестирования не сильно коррелирует размером кусочков. Пишешь по тесту на 1-2 метода некоего класса и пофиг что в нём 200 методов. Не факт, что иметь 100 классов с 2 методами лучше, чем 1 класс с 200.

A>Решил вот поучиться писать юнит-тесты. Придумал такой кусок кода в качестве "дано":

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

Здравствуйте, -VaS-, Вы писали:

A>>Решил вот поучиться писать юнит-тесты. Придумал такой кусок кода в качестве "дано":

VS>Не с той стороны заходишь. Начинать надо было с юнит-теста. Думаю, ты уже начал догадываться, что основная цель того, что ты делаешь — не "покрыть код тестами", а получить нормальный дизайн. Покрытие — лишь побочный эффект.

На моё "дано" можно смотреть как на постановку задачи, а не как на готовый код. Фактически — вместо того, чтобы писать "я хочу написать сервис, который блаблабла, набигают охранники, блабла, можно грабить корованы" я написал этот код. Слов меньше, суть понятнее. На этот код можно забить, сделать вид, что его нет. Я с радостью выслушаю мысли на тему того, каким должен быть самый первый тест и что именно он должен тестировать. Мне пока даже зацепиться не за что — есть опыт только с интеграционными тестами.

A>Привет
Привет.

A>и в качестве задачи — покрыть его функциональность тестами. Не интеграционными. Чтоб без базы.
У кода всего 2 зависимости: userRepository и noteRepository. Заменить их на интерфейсы и подменять (мокать) во время тестов.
По поводу техник избавления от зависимостей очень рекомендую книгу Эффективная работа с унаследованным кодом.



Все остальное мало связано с тестированием. А только с организацией кода. Т.е. архитектурой.
A>Решение в лоб, которое приходит в голову:
A>1. Из блока №1 сделать класс вроде UserByUserIdResolver, тестировать его отдельно от всего.
Логичнее поместить его в userRepository.
A>2. Из блока №2 сделать NoteFactory, с методом makeNote(User owner, String title, String text), тестировать отдельно от всего.
Судя по сигнатуре эта фабрика будет использоваться только здесь. Зачем ее выделаять в класс?
A>3. Из блока №3 сделать что-то вроде UserActivityUpdater, тоже тестировать отдельно.
Этот кусок нужно выводить в инфраструктуру. Иначе он бдует в каждом методе.
A>4. Из блока №4 сделать класс NoteToNoteDTOMapper, ну и тоже отдельно.
Только если этот маппер нужен где-то еще.

A>Из плюсов: все эти куски можно будет тестировать совершенно независимо, т.к. реально они отвечают за разные аспекты поведения. Код самого метода станет значительно короче и читабельнее.
Не всегда. В данном случае, ИХМО, все наоборот.
Блок 1 лучше чем UserByUserIdResolver. Эти строчки читаются и анализируются за пол ссекунды. Потому что это общеизвесный паттерн. А над вызов UserByUserIdResolver нужно будет подумать.
A>Из минусов: это реально дохрена каких-то шумоподобных классов по 3 строчки. UserByUserIdResolver... что это за нафиг вообще? (в смысле — я недоволен смысловой нагрузкой абстракции, ощущается как TwoPlusTwoCalculator)
+100

СУВ, Aikin

A>На моё "дано" можно смотреть как на постановку задачи, а не как на готовый код. Фактически — вместо того, чтобы писать "я хочу написать сервис, который блаблабла, набигают охранники, блабла, можно грабить корованы" я написал этот код. Слов меньше, суть понятнее.

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

А>На этот код можно забить, сделать вид, что его нет. Я с радостью выслушаю мысли на тему того, каким должен быть самый первый тест и что именно он должен тестировать.

Попытаюсь угадать требования:

1. Сохранить в базе новую заметку пользователя.
2. Обновить время последнего действия пользователя.
3. Уведомить заинтересованные объекты о п. 1.

Начинаем с первого пункта:

[Test]
public void Saves_new_note_for_given_user()
{
  const long userId = 2;
  const string title = "title";
  const string text = "text";
  var user = new User();
  var users = Stub<Users>();
  var notes = Mock<Notes>();

  SetupResult.For(users.Find(userId)).Return(user);
  Expect.Call(() => notes.CreateNew(user, title, text));

  Replay();

  var creator = new NotesCreator(users, notes);
  creator.Create(userId, title, text);
}

Почему так? Куда делся "сервис", "репозитории" и даже DTO? Да потому что все это детали реализации, которые к домену не имеют никакого отношения. Я хочу видеть Пользователей, Заметки и Создателя Заметок. DTO появится потом и не как возвращаемое значение.
Пишем код. Тест проходит. Пишем тесты Updates_user_last_activity_after_creating_new_note, Notifies_listener_after_creating_new_note, Throws_exception_if_user_not_found и код под них, вводя новые зависимости, объеденяя в более крупные, если их количество становится настораживающим и т.д. Все время смотрим на код, смотрим на тесты, рефакторим пока и то, и другое не будет вызывать чувство удовлетворенности

.> Не факт, что иметь 100 классов с 2 методами лучше, чем 1 класс с 200.

Еще какой факт. Нарушение SRP со всеми вытекающими последствиями. Если, конечно, твой "класс" не класс вовсе, а просто-напросто нэймспейс для кучи процедур/функций. Но автор ведет речь об ООП.

A>И вот вопрос — какими ценностями стоит руководствоваться, если хочется чтобы ВСЁ легко было протестировать. Отключив мозг я понимаю, что если порезать логику достаточно мелко, тестировать будет легко. Включив мозг я понимаю, что порезать логику достаточно мелко — значит утонуть в болоте из этой логики, потому что навскидку не так просто будет сказать "что же вообще делает эта программа и как она это делает", т.к. одна банальная операция создания новой заметки будет тянуть за собой в простейшем случае 5 объектов, а при добавлении новых требований, которые я описал выше их станет не менее 10. При этом тестироваться всё будет замечательно, но при это будет 10 классов. Для "почти" одного метода сервиса. Нет, они конечно будут реюзаться во всяких update/delete, но будет ли этот результат в итоге вменяемым?

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