Pull to refresh

Comments 12

Жуть какая :) Зачем это для каждого контроллера проверять то? Не думаю, что вы меняете эту функциональность в каждом контроллере.
Кстати, судя по направлению, избранному командой RoR, спеки контроллеров ждёт печальная участь.
Конечно, я не меняю эту функциональность в каждом контроллере, да и проверка в них сводится к тому, чтобы упомянуть методы в before_action. Но это надо не забыть сделать. Такой тест — хороший способ быть уверенным в том, что не забыл.

А можете просвятить, что это за направление, избранное командой RoR, из-за которого спеки контроллеров ждёт печальная участь?
Конечно, можно сделать одну проверку в коде или запихнуть before_action в ApplicationController (и добавить skip_before_action, где проверять не надо). Но это же implementation detail! Я хотел своего рода отчёт, который показывает, что (почти) каждый URL, которым приложение смотрит наружу, требует передачи access token. Не должны тесты знать, как устроена проверка внутри — они должны просто показывать, что всё работает, как ожидается. А как это сделать, не дёргая каждый URL?

В приведённом Вами тикете, кстати, DHH придерживается сходной точки зрения. Так что я грядущие изменения поддерживаю обеими руками.
Слышали, но на тот момент я уже был охвачен идеей написать мета-тест. Буду благодарен, если приведёте пример, как эту же проблему решить с помощью shared examples.
Из своего опыта скажу следующее:
1. Тесты контроллеров, как правило, не окупают себя — они пишутся дольше всего и наиболее чувствительны к изменением тестируемого кода. Лично по мне — в рельсах лучше максимально отказаться от тестов контроллеров (оставить только там где это необходимо), а писать интеграционные тесты — т.е. именно слать запросы get, put, post, delete и т.д. на нужные адреса, либо кликать по ссылкам.
Вы же не тестируете каждую вьюху? Хотя такая возможность тоже есть.
2. Лучше всё же shared examples. Решается просто — весь общий код пишется, собственно, в него, а нужные переменные передаются через let:
it_behaves_like "protected controller" do
  let(:method) { 'hello' }
end

По мне, основное и главное свойство тестов (помимо того что они тестируют нужный функционал) — это их читаемость.
Непонятные тесты, которые дают 100% покрытия — не тот результат, к которому бы я стремился. Понятные читаемые тесты, тестирующие большинство ситуаций + самые критичные места — это моя цель.
Спасибо, за то, что поделились своим опытом. Именно на это я и рассчитывал, публикуя сей опус. :-)

Тесты контроллеров, как правило, не окупают себя — они пишутся дольше всего и наиболее чувствительны к изменением тестируемого кода. Лично по мне — в рельсах лучше максимально отказаться от тестов контроллеров (оставить только там где это необходимо), а писать интеграционные тесты — т.е. именно слать запросы get, put, post, delete и т.д. на нужные адреса, либо кликать по ссылкам.

Полностью поддерживаю. Именно поэтому и появился этот мета-тест, ибо на мой взгляд проверка того, что приложение отфутболивает при отсутствии credentials, является crucial part, а остальное, может быть, и не надо совсем. Покроется интеграционными тестами. Другой вопрос, что и этот тест, вероятно, следовало бы сделать интеграционным, а не ориентированным на контроллеры, но суть от этого не меняется.

2. Лучше всё же shared examples. Решается просто — весь общий код пишется, собственно, в него, а нужные переменные передаются через let

Если я правильно понимаю, придётся тогда городить «расширение» этих shared examples для каждого контроллера, а я вообще не хочу писать спеки для контроллеров. Написал один раз и забыл (ну или почти забыл).

Непонятные тесты, которые дают 100% покрытия

То есть, этот тест непонятный? У меня-то как раз сложилось обратное представление: вся суть в самом последнем describe и двух context, и эти блоки почти ничем не отличаются от тех, что я писал отдельно для каждого контроллера. Разве что
 before { self.controller = c.new }

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

Вопрос читаемости кода очень субъективный. И автору этого кода нечитаемость становится очевидна обычно не сразу. Мне довелось не так давно встретитить вот такую конструкцию в тестах на TestNG, например:
            return new Object[][]{
                    {new LoginHelperTestDataProviderLoginAsUserValue("96968c29f29b96103b1c38f30d8f0fe2", 1l, 2l, Role.ROLE_SUPER)},
                    {new LoginHelperTestDataProviderLoginAsUserValue("d4bbe02790bdc33c5778fd8ec4e4d036", 2l, 5l, Role.ROLE_ADMIN)},
                    {new LoginHelperTestDataProviderLoginAsUserValue("712d3a285047d2ed252990d53cca3893", 2l, 56l, Role.ROLE_ADMIN)},
                    {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 3l, 232l, Role.ROLE_MANAGER)},
                    {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 4l, 64324l, Role.ROLE_MANAGER)},
                    {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 5l, 86456l, Role.ROLE_MANAGER)},
            };

Никакой другой реакции кроме WAT?! на это, по-моему, быть не может, хотя, уверен, автору казалось всё кристально чистым, когда он это создавал. А мне даже статью написать захотелось на тему, как загубить идею, ведь это уже не первое подобное использование концепции DataProvider, которое я встречаю.
Лично я бы предпочел метод self.excluded_actions заменить на константу EXCLUDED_ACTIONS. Это фиксированное значение, тут хэш не тянет никаких данных и не вызывает другие методы.
По-моему, то, что получилось — очень плохо. По нескольким причинам:

1. Плохо читаемый и громоздкий код.
2. Неиспользование стандартных инструментов фреймворка.
3. Слишком много ненужного мета-программирования (что на самом деле подмножество п.1).
4. Избыточное число тестов на выходе.

Я вижу два пути:
1. Или как советовали выше много раз использовать shared examples.
2. Или вынести функционал в ApplicationController (или в любой другой класс, например, SecureController и наследовать уже от него), протестировать этот функционал в одном месте с помощью Anonymous Controller. А затем в каждом конкретном случае проверять в тесте, что этот конкретный контроллер наследуется от ApplicationController (или, соответственно, SecureController).
Не очень понимаю зачем все это нужно, т.к. вы же всегда свои контроллеры для API наследуете от API::BaseController. Вот его-то и тестируйте один раз на все случаи аутентификации и авторизации.

Например так

require 'rails_helper'

describe Api::BaseController, type: :controller do

  subject { get :index }
  let(:user) { Fabricate(:user) }
  let(:admin) { Fabricate(:admin) }

  controller do
    def index
      render nothing: true, status: :ok
    end
  end

  describe 'when user isn\'t admin' do
    before {
      sign_in user
      subject
    }

    it 'return 403' do
      expect_status '403'
    end
  end

  describe 'when user is admin' do
    before { 
      sign_in admin
      subject
    }

    it 'return 200' do
      expect_status '200'
    end

  end

end
Sign up to leave a comment.

Articles