Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky specs using CKEditor #2711

Merged

Conversation

javierv
Copy link
Contributor

@javierv javierv commented Jun 29, 2018

References

Objectives

Fix the flaky specs that appeared in spec/features/ckeditor_spec.rb:14 ("CKEditor is present before & after turbolinks update page"), spec/features/debates_spec.rb:998 ("Debates Suggesting debates Shows up to 5 suggestions"), spec/features/debates_spec.rb:1019 ("Debates Suggesting debates No found suggestions"), spec/features/proposals_spec.rb:1597 ("Proposals Suggesting proposals No found suggestions"), spec/features/budgets/investments_spec.rb:728 ("Budget Investments Phase I - Accepting Suggest Show up to 5 suggestions"), spec/features/budgets/investments_spec.rb:1061 ("Budget Investments behaves like nested documentable at new budget investment path Should not update document cached attachment field after unvalid file upload"), spec/features/proposals_spec.rb:1440 ("Proposals behaves like nested imageable at new_proposal_path Should show new image after successful creation with one uploaded file"), spec/features/tags/budget_investments_spec.rb:120 ("Tags Turbolinks sanity check from budget's show"), spec/features/proposals_spec:1467 ("Proposals behaves like nested documentable at edit proposal path Should update loading bar style after valid file upload") and spec/features/tags/budget_investments_spec.rb:82 ("Tags Category with category tags").

Explain why the test is flaky, or under which conditions/scenario it fails randomly

Most assets are precompiled before running a Travis build. However, as mentioned in pull request #1196, precompiling all CKEditor plugins would take too long, and so they aren't included.

On the other hand, when no plugins are precompiled, the first time a page loading CKEditor is rendered, the application takes a few seconds to compile the plugins it uses. If there's an AJAX request during that time, the test will probably fail because the compilation will take longer than Capybara's wait time.

In a similar way, tests using fill_in_ckeditor may fail because this method is executed while the plugins are still being compiled. It results in the editor area not being filled properly, as shown in this screenshot taken by Capybara:

Description is empty

To reproduce one of these failures locally, run:

RAILS_ENV=test bundle exec rake assets:precompile
rm -r tmp/cache/assets/sprockets
rspec spec/features/ckeditor_spec.rb

If it still passes, maybe your machine is too fast 馃槂. Try setting Capybara.default_max_wait_time = 1, deleting the sprockets cache again, and running the spec again.

Explain why your PR fixes it

By precompiling only the CKEditor plugins the application uses, they don't need to be compiled in the middle of a test, AJAX calls don't exceed Capybara's wait time, and compiling CKEditor assets doesn't take too long.

@javierv javierv changed the title Fix flaky AJAX specs in Travis Fix flaky specs using CKEditor Jun 29, 2018
Copy link
Collaborator

@aitbw aitbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, @javierv ! 馃帀

I've been testing this locally (as well as restarting this PR's Travis' builds several times) and so far, so good!

Just one small thing: Can you squash these commits? Being such a small patch, there's no need to clutter Git history with multiple commits

Let me know what you think or if you need any help! 馃樃

As stated in consuldemocracy#1196, compiling everything related to CKEditor made
compilations slower. However, not compiling any plugins meant Travis had
to compile them while running a test. It often resulted in a test
failing because the time Travis took to compile the plugins the
application uses exceeded Capybara's wait time.
@javierv
Copy link
Contributor Author

javierv commented Jul 2, 2018

馃憣 Done!

@aitbw
Copy link
Collaborator

aitbw commented Jul 2, 2018

Great, thanks a lot @javierv ! 馃帀

I'll be testing this patch thoroughly later today and if all goes well, I'll let you know 馃樃

@javierv
Copy link
Contributor Author

javierv commented Jul 3, 2018

@aitbw I've added a Failed Tags Category with category tags spec to the list of flakies fixed by this pull request, with a screenshot showing the failure.

If there's anything I can help with, let me know! 鈽猴笍

@javierv
Copy link
Contributor Author

javierv commented Jul 3, 2018

@aitbw I've just added the failure shown in #2720 Travis build to the list of flakies fixed by this pull request.

P.S. Sorry I first added this comment to the wrong place!

@aitbw
Copy link
Collaborator

aitbw commented Jul 8, 2018

Hi @javierv ! First of all: Sorry for the late reply. We've been very busy lately.

As I told you a few days ago, I've been testing this thoroughly locally and on Travis and seems like all the scenarios you mentioned on the PR description (thanks a lot for keeping it updated) are completely free of flakiness 馃帀

I'll let the team know so we can get this merged ASAP 馃槍

Thanks a lot once again 鉂わ笍

@voodoorai2000
Copy link
Member

This is Fantastic! 馃帀
Thank you so much @javierv 馃憦

@voodoorai2000 voodoorai2000 merged commit f700f33 into consuldemocracy:master Jul 9, 2018
@javierv javierv deleted the fix_flaky_ckeditor_specs branch July 9, 2018 19:11
@aitbw aitbw mentioned this pull request Aug 23, 2018
Senen added a commit to rockandror/consuldemocracy that referenced this pull request Apr 22, 2020
This change adresses ithe problem with assets pipeline fallback on testing
environment described here: consuldemocracy#2711. Now no extra precompilation is done when
running ckeditor feature specs that uses javascript.
Senen added a commit to rockandror/consuldemocracy that referenced this pull request Apr 22, 2020
This change adresses ithe problem with assets pipeline fallback on testing
environment described here: consuldemocracy#2711. Now no extra precompilation is done when
running ckeditor feature specs that uses javascript.
javierm pushed a commit that referenced this pull request Apr 22, 2020
This change adresses ithe problem with assets pipeline fallback on testing
environment described here: #2711. Now no extra precompilation is done when
running ckeditor feature specs that uses javascript.
Senen added a commit to rockandror/consuldemocracy that referenced this pull request Apr 23, 2020
All of these plugins are not used anywhere.

Change introduced at ckeditor initializer will ommit unneeded
precompilation of plugins assets on production environments.

Change introduced at ckeditor config file adresses the problem with assets
pipeline fallback on testing environments described here: consuldemocracy#2711. Now plugins
that are explicitly disabled will not be precomiled when running ckeditor
javascript enabled feature specs.
@javierm javierm self-assigned this Nov 11, 2021
@javierm javierm added the UX label Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants