Василий Половнёв

Ведущий разработчик.

Специализация — Рельсы, фронтенд, автоматизация тестирования и разработки.

Связаться: vasily@polovnyov.ru или телеграм.

Блог Курс о тестах в Руби и Рельсах

Сервисы головного мозга

В понедельник встретил в коде такое:

# app/services/user_initials_extractor.rb
class UserInitialsExtractor
  def self.call(name)
    name
      .upcase
      .squeeze
      .split(" ")
      .map { |part| part[0] }
      .join(". ")
      .concat(".")
  end
end

# app/views/shared/_header.html.erb
<%= UserInitialsExtractor.call(current_user.name) %>

И меня тригернуло: это процедурное программирование, которое выдает себя за ООП, используя классы-помойки. Объекты — это живые организмы, которые обмениваются друг с другом сообщениями. UserInitialsExtractor — тупая процедура, завернутая в класс-сервис с убогим АПИ (#call, серьезно?).

Гораздо лучше было бы подумать, какой объект (существующий или новый) должен отвечать на #initials? Это мог бы быть User, декоратор для него или вообще отдельный объект, представляющий собой Имя:

class User
  # ...
  delegate :initials, to: :name
end

class UserName
  # ...
  def initials
  end
end

Чтобы не заниматься таким, советую прочитать: https://www.yegor256.com/2015/03/09/objects-end-with-er.html

«Что» и «Чтобы что» в пулреквестах

Несколько месяцев назад завел темплейт для пулреквестов с двумя вопросами: «Что?» и «Чтобы что?». Открываешь пулреквест, отвечаешь на вопросы и отправляешь ПР на ревью. Ревьюер сразу погружается в проблему, быстрее понимает, чего мы пытались достичь, и представляет образ решения.

Пара примеров:

## Что?
Обновляем Рельсы до 6.1.3.2.

## Чтобы что?
Чтобы устранить проблемы с безопасностью: CVE-123123.
## Что?
Перед отслеживанием купона проверяем, что он действительно существует в БД.

## Чтобы что?
Чтобы не записывать в аналитику всякий шлак, типа utm_campaign, utm_source или utm_medium.

Заметил еще и положительный эффект для автора пулреквеста: если не можешь внятно сформулировать «Чтобы что?», то не понимаешь задачу; если в «Что?» есть «А также» и «А еще», то лучше пулреквест разбить на несколько.

Короче, советую попробовать.

Где проходит грань в изолированности объекта тестирования?

Вопрос:

Где проходит грань в изолированности класса (объекта тестирования)? Как ты принимаешь решение что тестировать напрямую, а что стабами?

Ситуация: есть аудитория, которую можно импортировать из csv-файла. Чтобы проверить, что файл валиден, нужно убедиться, что в csv-файле есть колонки с почтой и именем клиента:

class AudienceList
  def valid?
    csv_headers.include?("Email") && csv_headers.include?("Name")
  end

  private

  def csv_headers
    @csv_headers ||= csv_data.first.keys
  end

  def csv_data
    @csv_data ||= SmarterCSV.process(file)
  end
end

AudienceList зависит от внешнего метода-запроса SmarterCSV#process. По умолчанию внешние зависимости я стаблю: это позволяет получить полностью изолированный, по-настоящему модульный, тест:

allow(SmarterCSV).to receive(:process)
  .and_return(sample_csv_data)

# ...

expect(audience_list).to be_valid

Но если я чувствую себя неуверенно с получившимся тестом, если чувствую, что без стаба тест будет надежнее и полезнее, то тестирую напрямую. В данном случае я бы заготовил два csv-файла (валидный и невалидный) и положил бы их в spec/fixtures. Эти файлы послужили бы еще и отличным источником примеров, документации. Разработчики смогли бы заглянуть в них и подсмотреть реальные данные и их структуру.

Только то, что важно для проверки

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

context "when coupon is applied" do
  it "charges user $500" do
    allow(Cashier).to receive(:charge)

    purchase.perform

    expect(Cashier).to have_received(:charge)
      .with(
        user: user,
        sum: 500,
        description: "Покупка абонемента",
        coupon: coupon
      )
  end
end

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

context "when coupon is applied" do
  it "charges user $500" do
    allow(Cashier).to receive(:charge)

    purchase.perform

    expect(Cashier).to have_received(:charge)
      .with(hash_including(sum: 500))
  end
end

Меньше — лучше

Чем меньше пулреквест, тем лучше: легко отревьюить, легко задеплоить, легко искать ошибки. Чем больше пулреквест, тем сильнее дизмораль: увидел +700 -60 в Гитхабе и приуныл.

Кроме того, маленькие пулреквесты дают параллелить задачу: собрали письмо-заглашку, показали дизайнерам, пока дизайнеры смотрят — делаем отправку этого письма. Один пулреквест — одна атомарная задача.

Конечно, бывают сложные ситуации. Например, апгрейд Рельс. Одна огромная задача с кучей изменений. Но даже тут можно обойтись крошечными пулреквестами:

1. Открываем ветку rails-6-upgrade.

2. Делаем маленькие пулреквесты не к master, а к rails-6-upgrade.

3. Ревьюим, мержим их в rails-6-upgrade.

4. Когда апгрейд готов, мержим rails-6-upgrade в master.

Пора валить?

Худшее, что может сделать программист для своей карьеры — обменять десять лет своей жизни не на десять лет опыта, а на один год, повторенный десять раз. Вот сидишь ты в собственном кубикле, программируешь там, тут остановился и подумал: Ух, ты! Ну и псих же я: десять лет на одном месте с тем же самым Друпалом. Бывает такое?

Чтобы вовремя заметить такое болото, раз в полгода я задаю себе три вопроса:
1. Что интересного, значительного и важного я сделал за прошедшие полгода?

2. Что за навыки я получил или прокачал за прошедшие полгода? Они вообще мне нужны?

3. Как мне команда и компания? Что с доверием, ценностями и миссией компании? Я еще верю в них? Или уже все пронизано цинизмом и пессимизмом?

Если ответы по двум из трех вопросов удручающие, пора валить.

Еще по теме:
How to waste your career, one comfortable year at a time

Когда использовать double, а не instance_double?

Напомню разницу: instance_double может уронить тест, если застабленные методы отсутствуют в указанном классе, double на все пофиг.

По моему опыту double нужен в двух случаях:
1. Вместо объекта, который пока не существует в системе. Нет класса, значит, instance_double не на что опереться.

2. Вместо чего-то незначительного со стабильным АПИ. Например, для писем:

allow(DeadlineMailer)
  .to receive(:last_deadline_warning)
  .and_return(double(:email, deliver_later: true))

RSpec: before и after хуки

Почему-то сталкиваюсь с такими тестами:

describe "#foo" do
  before :each do
    # ...
  end
end

:each можно смело опускать: это поведение по умолчанию для before. Лучше так:

describe "#foo" do
  before do
    # ...
  end
end

И несколько интересных фактов о before и after хуках:

1. before :each и before :all — алиасы для before :example и before :context.

2. before :each выполняется перед каждым примером, it do...end. after :each — после.

3. before :all выполняется перед контекстом (context, describe). after :all — после.

4. В before :suite нельзя задавать переменные экземпляра (instance var, @foo)

5. Только в before :each можно мокать.

Как застабить переменные окружения в RSpec

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

# Если в коде ENV["CHARGES_TOKEN"]
allow(ENV)
  .to receive(:[])
  .with("CHARGES_TOKEN")
  .and_return("XXX")

# Если в коде ENV.fetch("CHARGES_TOKEN")
allow(ENV)
  .to receive(:fetch)
  .with("CHARGES_TOKEN")
  .and_return("XXX")

Если вы сторонник готовых гемов, возьмите для этой цели ClimateControl:

ClimateControl.modify CHARGES_TOKEN: "XXX" do
  # ...
end

Что не нужно писать в it

1. Бесполезные, общие слова, не несущие никакой конкретики:

it "adds certain value"
it "returns correct result"
it "fails"
it "returns formatted string"
it "returns correct url"
it "is ok"

2. Детали реализации:

it "changes @scheduled_on"
it "assigns @todos"

3. Ложь:

it "returns time in 24-hour format" do
  expect(...).to eq "9:25"
end

it "strips leading zeroes" do
  expect(foo(" 9:25 ")).to eq "9:25"
end

И, пожалуйста, не тестируйте конструкторы и attr_reader/writer/accessor: вы все равно их проверите, тестируя публичный АПИ.

RuboCop, RSpec и стайлгад

Годами стайлгайдом для RSpec был betterspecs.org. К сожалению, он годами не менялся, не развивался и частенько не работал.

Оказывается, 1,5 года назад Better Specs стал RSpec Style Guide и переехал в Rubocop HeadQuarters:
https://github.com/rubocop-hq/rspec-style-guide

И стайлгайд ожил и расцвел:
https://rspec.rubystyle.guide

И, конечно, есть плагин к Рубокопу для работы со спеками:
https://github.com/rubocop-hq/rubocop-rspec

Подключается в два счета:

# Gemfile
gem "rubocop-rspec", require: false

# .rubocop.yml
require:
  - ...
  - rubocop-rspec

Полный список копов:
https://docs.rubocop.org/rubocop-rspec/cops_rspec.html

Как протестировать конфиг whenever

Недавно я опечатался в конфиге whenever:

every 1.day, at: "03:30 am", roles: %i(backupable) do
  rake %(
    backup:db
    backup:assets
  ).join(" ")
end

При деплое whenever взорвался:

NoMethodError: undefined method `join' for "backup:db backup:assets":String

Чтобы в будущем такого не было, нужна хотя бы минимальная валидация конфига whenever. Решение оказалось простым: достаточно запустить на CI:

bundle exec whenever

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

Один тест — одна логическая проверка

Проверка — фаза теста, в которой мы сверяем результат испытаний с правильным:

expect(post.slug).not_to be_nil
expect(account.admins).to match_array(admins)
expect(comment_notification).to have_received(:deliver)
expect { post.destroy }.to change(Post, :count).by(-1)

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

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

describe "#color" do
  it "is blue" do
    expect(color.R).to be < 0.2
    expect(color.G).to be < 0.2
    expect(color.B).to be > 0.8
  end
end

В таких случаях лучше объединить проверки в одну с помощью have_attributes:

describe "#color" do
  it "is blue" do
    expect(color).to have_attributes(
      R: (a_value < 0.2),
      G: (a_value < 0.2),
      B: (a_value < 0.8),
    )
  end
end

А если проверить нужно несколько связанных сайд-эффектов, лучше объединить их с помощью and:

it "activates subscription" do
  expect { subscription.activate! }
    .to change { subscription.reload.active? }.to(true)
    .and change { subscription.reload.valid_until }.to(1.month.from_now)
end

Обратная связь в тестах, guard и vim-test

Я фанат мгновенной обратной связи в тестах: чем быстрее я получу результаты теста, тем быстрее вернусь к коду, тем быстрее сделаю работу. Для этого использую вотчеры: в Руби Guard отслеживает изменения в файлах и автоматически запускает нужные спеки, в JS это делает jest –watch.

Постепенно они стали меня бесить: сохраняешь файл, тест прогоняется через две секунды. Эти две секунды — целая вечность. Часто думаешь, что не сохранил файл, сохраняешь еще раз. Тест прогоняется еще пару раз. Да еще и целиком!

Постепенно перешел к ручному, но мгновенному запуску тестов с помощью vim-test:
https://github.com/janko/vim-test

Идея простая: сохранил спеку, нажал «пробел t» (пробел — это leader), получил терминал с тестом. Мгновенная обратная связь, советую.

P. S. Для запуска тестов использую вот такие шорткаты:

nnoremap <silent> <Leader>t :TestFile<CR>
nnoremap <silent> <Leader>s :TestNearest<CR>
nnoremap <silent> <Leader>l :TestLast<CR>

Не проверяйте отдельно ключи

Бывает, встречаю в спеках три отдельных теста на то, что полученный хэш в порядке: есть ключи, есть значения, у значений правильный тип.

RSpec.describe UserSerializer do
  describe "#as_json" do
    let(:user) do
      build(:user,
        first_name: "Bart",
        last_name: "Simpson",
        tel: "+777123")
    end
    let(:json) { described_class.new(user).as_json }

    it "has keys" do
      expect(json).to include(:first_name, :last_name, :tel)
    end

    it "has types" do
      expect(json[:first_name]).to be_kind_of(String)
      expect(json[:last_name]).to be_kind_of(String)
      expect(json[:tel]).to be_kind_of(String)
    end

    it "has values" do
      expect(json[:first_name]).to eq(user.first_name)
      expect(json[:last_name]).to eq(user.last_name)
      expect(json[:tel]).to eq(user.tel)
    end
  end
end

Такая спека скорее вредит, чем помогает: при изменении полей придется обновлять три теста; тяжело сразу понять, что именно возвращает сериалайзер. Лучше использовать лаконичный вариант:

RSpec.describe UserSerializer do
  describe "#as_json" do
    it "includes first name, last name and tel" do
      user = build(:user,
        first_name: "Bart",
        last_name: "Simpson",
        tel: "+777123")
      json = described_class.new(user).as_json

      expect(json).to include(
        first_name: "Bart",
        last_name: "Simpson",
        tel: "+777123"
      )
    end
  end
end

Не тестируйте attr_reader/_writer/_accessor

Регулярно встречаю тесты, в которых проверяют attr_reader:

class Alarm
  attr_reader :at

  def initialize(at: )
    @at = at
  end

  def snooze
    # ...
  end
end

RSpec.describe Alarm do
  describe "#at" do
    it "returns alarm time" do
      time = Time.now
      alarm = described_class.new(at: time)

      expect(alarm.at).to eq time
    end
  end
end

Это, конечно, бесполезно. Во-первых, так мы тестируем стандартную библиотеку — штуку, у которой уже есть собственные тесты. Зачем ей еще тесты?

Во-вторых, в таких тестах нет пользы, они проверяют тривиальное поведение.

В-третьих, геттер (Alarm#at) мы косвенно проверим в других тестах:

describe "#snooze" do
  it "snoozes alarm for nine minutes" do
    # ...

    expect { alarm.snooze }.to change { alarm.at }.by(9.minutes)
  end
end

P. S. ПРОТИП: если attr_reader нужен только самому объекту, его лучше сделать приватным:

private

attr_reader :repeat

Ищу сообразительного стажёра

Меня зовут Василий Половнёв, я веб-разработчик. Ищу сообразительного стажёра, который вместе со мной будет делать крутые бэкендерские штуки и со временем станет тимлидом.

Я ищу стажёра, который:

  • отличает GET от POST, SELECT от UPDATE;
  • отличает git rebase от git merge, пулреквест от реверта;
  • знаком с Рельсами на уровне «написал блог за 15 минут, а потом пилил его по выходным»;
  • слышал про RSpec, модульные и интеграционные тесты;
  • не боится ни легаси, ни ПХП, ни фронтенда.

За полтора месяца научу стажёра:

  • проверять свою работу модульными и интеграционными тестами;
  • писать тесты быстрее, чем код;
  • ревьюить, парно программировать и не унывать;
  • быстро работать с Гитхабом, Рельсами и Семафором;
  • находить и исправлять «запахи» в коде: от Long Method до Shotgun Surgery.

Для этого стажёр будет:

  • работать 20 часов в неделю, часть времени — в паре со мной;
  • тратить 2-3 часа в неделю на теорию и домашние задания;
  • переписывать код снова и снова после ревью.

После стажировки мы либо продолжим работать вместе, либо стажер уйдёт на майские каникулы с планом роста и списком рекомендуемой литературы.

Если написанное выше про вас, напишите мне письмо с рассказом о себе на vasily@polovnyov.ru.

Если нет, поделитесь постом в соцсетях. Вот картинка для Фейсбука:

Только то, что влияет на проверку

Смотрите:

it "builds tag slug from its title" do
  tag = Tag.new(name: "Веб-разработка", color: "#ccc")

  expect(tag.slug).to eq "veb-razrabotka"
end

Зачем тут color? На что он влияет? Как связан со slug и name? Чтобы ответить на все эти вопросы, придется покопаться в коде.

Бывает обратная ситуация:

it "builds tag slug from its title" do
  tag = build(:tag)

  expect(tag.slug).to eq "veb-razrabotka"
end

Почему veb-razrabotka? Откуда? От чего зависит? Чтобы ответить на эти вопросы, придется покапаться в фабриках.

Отсюда правило: в тесте должны быть данные, которые непосредственно влияют на проверку. Все остальное — под нож.

Какой тест упадет, если удалим этот кусок кода?

Смотрите, есть такой будильник:

class Alarm
  def to_human
    at.strftime("%k:%M").strip
  end
end

И такой тест:

it "returns alarm in human-readable 24-hour format" do
  time = Time.local(2019, 7, 7, 10, 35)
  alarm = described_class.new(at: time)

  expect(alarm.to_human).to eq "10:35"
end

Чтобы понять, достаточно ли этих тестов, я смотрю тестируемый модуль и спрашиваю себя: что тут особенного, если этот кусок кода удалить, какой тест упадет? Если никакой, то у меня 100% не хватает тестов или проверок.

В примере с будильником под вопросом .strip. Если удалить его, тест останется зеленым. Значит, не хватает теста:

it "strips any leading spaces" do
  time = Time.local(2019, 7, 7, 7, 15)
  alarm = described_class.new(at: time)

  expect(alarm.to_human).to eq "7:15"
end

Как отлаживать странное: в браузере на реальном Айфоне через Скайп, Зум или Хэнгаут

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

Чтобы не пересказывать коллеге то, что происходит на экране телефона, можно расшарить его экран:

  1. Открываем QuickTime
  2. File — New Movie Recording
  3. В выпадайке у кнопки записи выбираем Camera — iPhone
  4. Скдыщ!