Наверняка я не открою ничего нового для большинства тех, кто давно использует Go в работе. Но, зачастую оказывается, что люди не в курсе этого и мне будет проще отправлять их по ссылке, чем повторять из раза в раз одно и то же. Заодно может ещё кому-то будет полезно.
Дело вот в чём.
Допустим у нас есть структура с методами A, B, C. Но вот вдруг мы должны сделать вызов C из B, а ещё лучше, если появляется метод D и последовательность вызовов становится D->A + D->B->C в одном флаконе. В общем, – вложенные вызовы.
Если вложенные вызовы не изолировать, то тесты станут заметно длиннее и мы будем тестировать одно и то же в тестах разных методов.
Ситуация в коде:
package example
import (
"github.com/google/uuid"
)
//go:generate mockgen -source example.go -destination gomocks/example.go -package gomocks
type Dependency interface {
DoSomeWork(id uuid.UUID)
DoAnotherWork(id uuid.UUID)
DoAnotherWorkAgain(id uuid.UUID)
}
type X struct {
dependency Dependency
}
func NewX(dependency Dependency) *X {
return &X{dependency: dependency}
}
func (x *X) A(id uuid.UUID) {
x.dependency.DoSomeWork(id)
}
func (x *X) B(id uuid.UUID) {
x.dependency.DoAnotherWork(id)
x.C(id)
}
func (x *X) C(id uuid.UUID) {
x.dependency.DoAnotherWorkAgain(id)
}
func (x *X) D(id uuid.UUID) {
x.A(id)
x.B(id)
}
Обратите внимание на метод D. Он порождает длинные цепочки вызовов.
Теперь давайте представим, как может выглядеть тест метода D:
package example_test
import (
"testing"
"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/suite"
"example"
"example/gomocks"
)
func TestX(t *testing.T) {
suite.Run(t, new(XTestSuite))
}
type XTestSuite struct {
suite.Suite
ctrl *gomock.Controller
dependency *gomocks.MockDependency
x *example.X
}
func (s *XTestSuite) SetupTest() {
s.ctrl = gomock.NewController(s.T())
s.dependency = gomocks.NewMockDependency(s.ctrl)
s.x = example.NewX(s.dependency)
}
func (s *XTestSuite) TestD() {
var id = uuid.MustParse("c73d6461-f461-4462-b1fe-0aa9b500f928")
// Мы тестируем правильность работы не совсем тех методов, которые
// мы тестируем сейчас, но и всех остальных методов. Мы прогоняем
// всю логику насквозь. В ситуации, когда методы содержат десятки
// вызовов и более-менее сложную логику, это становится похоже
// на нетестируемый код из-за слишком высокой цикломатики.
s.dependency.EXPECT().DoSomeWork(id)
s.dependency.EXPECT().DoAnotherWork(id)
s.dependency.EXPECT().DoAnotherWorkAgain(id)
s.x.D(id)
}
Из этой ситуации есть простой выход. Что если изолировать X методы от самих себя?
Давайте добавим некоторые улучшения в наш код:
package example
import (
"github.com/google/uuid"
)
//go:generate mockgen -source example.go -destination gomocks/example.go -package gomocks
type (
Dependency interface {
DoSomeWork(id uuid.UUID)
DoAnotherWork(id uuid.UUID)
DoAnotherWorkAgain(id uuid.UUID)
}
This interface {
A(id uuid.UUID)
B(id uuid.UUID)
}
)
type X struct {
dependency Dependency
this This
}
type Option func(x *X)
func WithThisMock(this This) Option {
return func(x *X) {
x.this = this
}
}
func NewX(dependency Dependency, opts ...Option) *X {
x := &X{dependency: dependency}
for _, f := range opts {
f(x)
}
if x.this == nil {
x.this = x
}
return x
}
func (x *X) A(id uuid.UUID) {
x.dependency.DoSomeWork(id)
}
func (x *X) B(id uuid.UUID) {
x.dependency.DoAnotherWork(id)
x.C(id)
}
func (x *X) C(id uuid.UUID) {
x.dependency.DoAnotherWorkAgain(id)
}
func (x *X) D(id uuid.UUID) {
// Изолировали вложенные вызовы.
x.this.A(id)
x.this.B(id)
}
Что мы тут сделали? Мы изолировали вызовы методов типа X из его же методов. Теперь мы можем написать тест метода D тестируя только логику метода D.
Смотрим на тест:
package example_test
import (
"testing"
"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/suite"
"example"
"example/gomocks"
)
func TestX(t *testing.T) {
suite.Run(t, new(XTestSuite))
}
type XTestSuite struct {
suite.Suite
ctrl *gomock.Controller
dependency *gomocks.MockDependency
this *gomocks.MockThis
x *example.X
}
func (s *XTestSuite) SetupTest() {
s.ctrl = gomock.NewController(s.T())
s.dependency = gomocks.NewMockDependency(s.ctrl)
s.this = gomocks.NewMockThis(s.ctrl)
// В рабочем коде мы можем использовать
// конструктор как example.NewX(realDependency).
s.x = example.NewX(s.dependency, example.WithThisMock(s.this))
}
func (s *XTestSuite) TestD() {
var id = uuid.MustParse("c73d6461-f461-4462-b1fe-0aa9b500f928")
// Теперь мы тестируем только метод D.
s.this.EXPECT().A(id)
s.this.EXPECT().B(id)
s.x.D(id)
}
Всё сильно упростилось, верно?
Надеюсь это будет полезно мне самому и мне больше не придётся повторять это на словах, а так же ещё кому-то, кто ещё не в теме. :)
По поводу самого слова this. Это наверно не совсем идиоматично, но можно использовать любое другое слово, например self или ватева.
Комментарии (9)
manyakRus
04.05.2022 10:56Код был простой и понятный (39 строк) и тест тоже (44 строк)
Стало слишком сложно 65+43 строк = +30% кода
как будто тесты важнее самой программы
iamwizard
05.05.2022 23:20+1Мне кажтеся, что если тест писать неудобно - значит что-то не так со структурой кода.
В приведенном вами примере, метод D можно выделить в отдельную сущьность и передавать ей снаружи в качестве зависимости нечто реализующие A() и B()
Суть та-же, но будет нагляднее.
powerman
Не верно.
Вы перешли от тестирования поведения к тестированию реализации (т.е. получили более хрупкий тест, который будет ломаться при рефакторингах внутреннего устройства тестируемого объекта, даже если его внешнее API и поведение не меняется).
Код стал менее очевиден и более подвержен ошибкам (легко забыть сделать вызов через
.this, потому что и без него всё сработает не хуже).В публичное API тестируемого типа вылезли ручки связанные с тестированием. Этого не всегда получается избежать, но во-первых тут точно нечем гордиться, и во-вторых надо хотя бы стараться их маскировать под штатный функционал (хотя бы в теории полезный не только в тестах).
Хуже всего, что Вы предлагаете это решение как паттерн, что может привести к большой куче
.thisв разных проектах, от чего их код явно не станет лучше. Правильные тесты должны делать код и архитектуру чище, а не создавать лишние нагромождения внутри тестируемого кода.Обычно желание реализовать нечто подобное возникает из-за проблемы совсем в другом месте: тестируемый объект слишком сложен и запутан, насколько, что тестировать его целиком, даже при наличии моков всех используемых им объектов, стало слишком сложно. Решением этой проблемы является рефакторинг сложного объекта пока он не развалится на несколько достаточно простых.
krocos Автор
Да нет, всё верно. ????
Не вылезли, опять же, они там были. Если держаться в рамках статьи.
В примере разве сказано, что у нас есть неэкспортированные методы, которые мы сделали экспортированными? Нет. Получается, что у нас были уже экспортированные методы. Значит они уже с тестами.
А отсюда получается, что если надо сделать дополнительные методы, которые по какому-то стечению обстоятельств среди прочего вызывают другие экспортированные методы, но не изолировать эти вызовы, то мы получим тестирование одного через другое. Другими словами тестировать A, B, C через D, когда тесты для A, B, C уже есть. И вот это – неправильно и хрупко.
Как это можно забыть, если вы этот вызов изолируете и ожидаете вызов в тесте?
Даже в мыслях не было.
Ну и в целом, да, я согласен вот с этим вот всем, что плохо, когда вы выносите на верх, тестируете детали реализации, про то, что надо хорошо рефакторить, правильно разделять и бла-бла.
Но эта статья не про это.
Эта статья про изоляцию вложенных вызовов. Это про приём избавляющий от проблем, когда вы имеете на руках вложенные вызовы.
Вот именно. Да.
powerman
Я вроде держусь в рамках. Наружу из полезного для тестов торчал только параметр
Dependency. В целом, принимать значения как интерфейсы а не конкретные типы - это хорошая практика и полезна она не только для тестов, так что это "ручкой для тестов" я не считаю. А вот добавленная экспортируемая функцияWithThisMock- это совершенно однозначно ручка для тестов, не имеющая иного применения.Это - абсолютно правильно. Коль уж они все экспортируемые и являются частью внешнего API этого пакета, то тесты каждого из них нужны для проверки поведения публичного API этого пакета.
В теории. На практике, если содержимое D это пара очевидных строк вроде
return A(B(C())), то можно тесты D не писать, заменить их более внимательным ревью. А если в D что-то не столь очевидное,ifкакой-нибудь завёлся, например, то можно ограничиться всего парой тестов, которые проверят инварианты этогоif. Но, опять же, на практике такие простые методы целиком состоящие из вызовов соседних экспортируемых - это редкость, обычно так пишут только хелперы-ленивчики, чтобы упростить выполнение типовой последовательности вызовов нескольких методов API. В противном случае это скорее признак того, что A, B и C (или некоторые из них) вообще не должны быть экспортируемыми, и их выставили наружу по причине неправильно спроектированного API этого пакета, и, по факту, это внутренние методы, к которым вообще не должно быть доступа извне (и в этом случае тесты надо было бы писать именно на D, а не внутренние методы).Причин две:
Тесты проверяют далеко не 100% логики, и конкретно это место могли пропустить.
Забытый
.thisв тесте отражается в забытом.EXPECT... казалось бы, каковы шансы забыть и то, и другое?.. :) На мой взгляд - довольно высоки, потому что многие разработчики при написании тестов добавляют.EXPECTуже после того, как только что написанный тест падает.Тут приём == паттерн. Вы же не говорите, что это приём, который уникально подошёл конкретно вашему проекту, и то только для одного конкретного пакета. Вы предлагаете это как приём везде, где появляется много вложенных вызовов - т.е. есть проблема общего вида и её универсальное решение. Если это не паттерн, то я прям не знаю.
И нет, он не избавляет от проблем. Для меня этот приём звучит больше похожим на другой известный "приём": идею тушить пожар бензином.
А вот "рефакторить и правильно разделять" - это не "бла-бла"! Это и есть наша работа: программирование. (Да, я в курсе, что в последние годы появилось немало "программистов", которых быстро научили пилить круды, но которые по факту программистами не являются. И ожидать от них, что они будут адекватно рефакторить, правильно разделять области ответственности, и умело бороться со сложностью - как минимум наивно. Но это не отменяет того факта, что есть и нормальные программисты, для которых всё это не "бла-бла", а вполне понятная и важная часть ежедневной работы.) Да, не всегда получается понять как всё упростить за приемлемое время. Но это значит только то, что либо надо выбить больше времени на эту задачу у бизнеса, либо поручить её более квалифицированному разработчику (предварительно наняв его при необходимости), либо проект начнёт тонуть под грузом сложности, из-за чего сильно замедлится скорость добавления новых фич для бизнеса плюс скоро с него начнут разбегаться разработчики.
Невозможно бороться со сложностью добавляя ещё больше сложности - а Вы предлагаете именно такой подход.
powerman
Есть, кстати, хорошая иллюстрация (из статьи Software Dark Ages) типичного замкнутого круга, показывающая как избыток сложности в коде приводит к разбегающимся с проекта разработчикам:
krocos Автор
Выдохните уже ))
Вы тут тучи сгущаете. Я про приём изоляции, а вы уже про уход сотрудников из-за повышенной сложности.
Какой-то это перегиб как по мне.