18 sphinx by geneux · Pull Request #16 · geneux/deep_recursion

geneux · 2016-09-24T12:59:10Z

No description provided.

…inclusion to Capybara JS execution

geneux · 2016-09-24T21:27:59Z

spec/features/star_answer_spec.rb

      second_answer = create(:answer, question: question, user: user)
      visit question_path(question)
      within("#answer-#{second_answer.id}") { click_on 'Star' }
+      sleep 0.1


Я не совсем понял, из-за чего точно, но таймаут капибары перестал успевать примерно на 100мс. Видимо, как раз в это время Сфинкс перестраивает дельта-индексы. Считаю, что костыль, но ничего умнее придумать не смог. И почему-то проявилось только в этом тесте.

Ох, ну тут с тестом проблемка, конечно. Зачем нам проверять id элемента? Это уже не пользовательский сценарий, а кодерский. Надо проверять контент, используя have_content, который автоматически подождет до появления контента и избавить от костыля в виде sleep.

Да, правда, можно было проще проверить. И, действительно, после переделки на have_content стало дожидаться. Не задумался, видимо, когда писал, что ждет именно он.

Вот рано я обрадовался. Вызов have_content вообще не помогает, ну, точно Сфинкс перестраивает дельты и блокирует объект в транзакции, его надо еще ждать.

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

В итоге вернул sleep 0.1, так как единственный вариант получается -- делать флаг ожидания индексации Сфинкса или завершения транзакции, и все равно делать в спеке вызов ожидания. Так как готовность Сфинкса и так проверяется через 100мс, то пусть уж лучше тут будет sleep, медленнее от него точно не станет по сравнению с предложенным методом.

have_content сам за тебя ждет, столько, сколько нужно. Ты как-то неправильно его используешь, значит. Ты, вероятно, оставил что-то вроде expect(page.first('div')).to have_content(...)?

Нужно так:

# сужаем область, в которой будет производится поиск элементов within "#answers div:first-child" do # а лучше использовать имя класса "#answers .answer:first-child" expect(page).to have_content(...) end

Когда ты делаешь page.first(...), то возвращается тот элемент, который был первым в момент вызова, и тут уже не помогает have_content.

Вообще, я это понял, исключил даже within и для проверки сделал вот так:

# sleep 0.1 expect(page).to have_content second_answer.body

И без комментарием тест работает, с комментарием -- валится. Но с #answers div:first-child мне вариант больше нравится, переделаю на него. C #answers .answer хоть и лаконичнее, но хуже читается, менее понятно, что выделяется.

Попробовал, не получается ни с #answers div:first-child, ни c #answers .answer (у меня в текущей ветке нет класса answer, но для эксперимента я его добавил). Всегда валится на Capybara::Ambiguous, например с: Ambiguous match, found 2 elements matching css "#answers .answer". Пробую из JS-консоли: один элемент. Странно.

В итоге sleep оставил, и have_content перед ним тоже, хотя, он не спасает совершенно. Думаю, варианты с селекторами, даже если он их смэтчит правильно, не пройдут тоже.

В итоге, это лучшее, что работает.

geneux · 2016-09-24T21:29:18Z

spec/support/shared_examples/controllers/sphinx.rb

+  end
+end
+
+shared_examples :sphinx_search_controller_with_params do |entity, attribute, search_type, validity = true|


Возможно, здесь с shared example-ом я немного "перехимичил", зато получилось весьма лаконичная сама спека

"Перехимичил", разве что, с расположением примеров.
В данном случае лучше сами shared_examples объявить в файле-спеке самого контроллера. Сейчас спека контроллера – сплошная магия, ее нельзя взять и прочитать, как спецификацию.

Самое смешное, что я так их и писал, потом выносил в отдельный файл. Именно, чтобы пока пишешь, не запутаться )). Понял, вернул в спеку.

geneux · 2016-09-24T21:34:15Z

app/models/search_service.rb

@@ -0,0 +1,28 @@
+class SearchService


Ну уверен, что правильнее: помещать сервисный объект в модели, или делать еще одну папку в /app и прописывать её в пути загрузки приложения? Пока объект всего один, положил в модели, чтобы не разветвлять дерево проекта.

Делать еще одну папку, app/services.

Ок, сделал, переместил.

palkan · 2016-09-25T09:43:19Z

app/models/search_service.rb

@@ -0,0 +1,28 @@
+class SearchService


Делать еще одну папку, app/services.

palkan · 2016-09-25T09:45:19Z

app/models/search_service.rb

+    private
+
+    def klass(type)
+      case type


Не очень мне нравится такой подход, у нас все-таки Ruby, можно применить что-нибудь из мета-программирования.
Например, получать с клиента не однобуквенные типы, а полные ("question", "answer"), и делать classify.constantize, предварительно фильтруя, конечно.

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

Сделал, спеки лаконичнее сильно не стали, но зато вынес в константу сервис-объекта допустимые значения.

palkan · 2016-09-25T09:48:23Z

spec/features/star_answer_spec.rb

      second_answer = create(:answer, question: question, user: user)
      visit question_path(question)
      within("#answer-#{second_answer.id}") { click_on 'Star' }
+      sleep 0.1


Ох, ну тут с тестом проблемка, конечно. Зачем нам проверять id элемента? Это уже не пользовательский сценарий, а кодерский. Надо проверять контент, используя have_content, который автоматически подождет до появления контента и избавить от костыля в виде sleep.

palkan · 2016-09-25T09:56:03Z

spec/support/shared_examples/controllers/sphinx.rb

+  end
+end
+
+shared_examples :sphinx_search_controller_with_params do |entity, attribute, search_type, validity = true|


"Перехимичил", разве что, с расположением примеров.
В данном случае лучше сами shared_examples объявить в файле-спеке самого контроллера. Сейчас спека контроллера – сплошная магия, ее нельзя взять и прочитать, как спецификацию.

geneux · 2016-09-25T22:14:13Z

Несмотря на проблему с тестом, в котором приходится спать 100мс, думаю, можно двигаться дальше, PR пока оставлю открытым, но в мастер зарибейжу. Если что надо будет поправить -- поправлю в следующих ветках или между ними.

… of sleep there

smartlime added 12 commits September 22, 2016 19:23

Install Sphinx

f8321c9

Add Sphinx generated config and db to .gitignore

3b5c3d8

Add Sphinx indices and some configuration

646905f

Draft search feature spec

9321893

Draft SearchController w/view and form in navbar

f3a784c

Fix Sphinx indices

7f8d0c1

Search implementation w/o specs and indices rebuild hooks

9de295f

WIP CommentsController ans specs

46b2bf2

Set up Sphinx Delta Indexes

7c54d83

SearchController with spec ans shared examples

98e8e0b

Fix broken star_answer spec because of strange side effect of Sphinx …

685d0c8

…inclusion to Capybara JS execution

Set up test suite for Sphinx, Add search related tests

b1d98df

geneux commented Sep 24, 2016

View reviewed changes

palkan reviewed Sep 25, 2016

View reviewed changes

smartlime added 3 commits September 25, 2016 16:09

Fix async waiting in star_answer_spec; Clarify test

f11749d

Move Sphinx search shared examples to tests

961399f

Move search/service to app/services

076016c

geneux force-pushed the 18-sphinx branch from b3a504c to d32d44a Compare September 25, 2016 13:49

Change search types to object names

812b0fb

geneux force-pushed the 18-sphinx branch from d32d44a to 812b0fb Compare September 25, 2016 14:06

smartlime added 2 commits September 26, 2016 00:35

Add default scopes of model for correct API lists ordering

05e76dc

Revert to delay in 'starred at top' feautre spec

4deeb64

geneux mentioned this pull request Sep 25, 2016

12 oauth #10

Open

geneux force-pushed the 18-sphinx branch from f089ce9 to 7ae42fc Compare September 25, 2016 22:23

Devise confirmation in User factory

73e5411

geneux force-pushed the 18-sphinx branch from 7ae42fc to 73e5411 Compare September 26, 2016 00:43

Call have_content in 'starred at top' feature spec but cannot get rid…

0ae29e2

… of sleep there

Conversation

geneux commented Sep 24, 2016

Uh oh!

geneux Sep 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geneux Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geneux commented Sep 25, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

geneux Sep 24, 2016 •

edited

Loading

geneux Sep 26, 2016 •

edited

Loading