Merge branch 'feature/reliable-rspec-with-eval-script' into 'master'

Check for element before execute_script

See merge request gitlab-org/gitlab-ce!14898

changelogs/unreleased/feature-reliable-rspec-with-eval-script.yml

@@ -0,0 +1,5 @@
+---
+title: Get true failure from evalulate_script by checking for element beforehand
+merge_request: 14898
+author:
+type: fixed

doc/development/testing_guide/best_practices.md

@@ -35,6 +35,8 @@ Here are some things to keep in mind regarding test performance:
   [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)).
 - Don't supply the `:each` argument to hooks since it's the default.
 - On `before` and `after` hooks, prefer it scoped to `:context` over `:all`
+- When using `evaluate_script("$('.js-foo').testSomething()")` (or `execute_script`) which acts on a given element,
+  use a Capyabara matcher beforehand (e.g. `find('.js-foo')`) to ensure the element actually exists.
 
 [four-phase-test]: https://robots.thoughtbot.com/four-phase-test
 

features/steps/project/issues/filter_labels.rb

@@ -28,12 +28,6 @@ class Spinach::Features::ProjectIssuesFilterLabels < Spinach::FeatureSteps
     end
   end
 
-  step 'I click link "bug"' do
-    page.find('.js-label-select', visible: true).click
-    sleep 0.5
-    execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()")
-  end
-
   step 'I click "dropdown close button"' do
     page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click
     sleep 2

features/steps/project/source/browse_files.rb

@@ -46,10 +46,6 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     expect(page).to have_content new_gitignore_content
   end
 
-  step 'I should see its content with new lines preserved at end of file' do
-    expect(evaluate_script('ace.edit("editor").getValue()')).to eq "Sample\n\n\n"
-  end
-
   step 'I click link "Raw"' do
     click_link 'Open raw'
   end
@@ -70,20 +66,11 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     click_link 'Fork'
   end
 
-  step 'I can edit code' do
-    set_new_content
-    expect(evaluate_script('ace.edit("editor").getValue()')).to eq new_gitignore_content
-  end
-
   step 'I edit code' do
     expect(page).to have_selector('.file-editor')
     set_new_content
   end
 
-  step 'I edit code with new lines at end of file' do
-    execute_script('ace.edit("editor").setValue("Sample\n\n\n")')
-  end
-
   step 'I fill the new file name' do
     fill_in :file_name, with: new_file_name
   end
@@ -395,6 +382,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   private
 
   def set_new_content
+    find('#editor')
     execute_script("ace.edit('editor').setValue('#{new_gitignore_content}')")
   end
 

spec/features/boards/boards_spec.rb

@@ -171,12 +171,14 @@ describe 'Issue Boards', :js do
         expect(page).to have_selector('.card', count: 20)
         expect(page).to have_content('Showing 20 of 58 issues')
 
+        find('.board .board-list')
         evaluate_script("document.querySelectorAll('.board .board-list')[1].scrollTop = document.querySelectorAll('.board .board-list')[1].scrollHeight")
         wait_for_requests
 
         expect(page).to have_selector('.card', count: 40)
         expect(page).to have_content('Showing 40 of 58 issues')
 
+        find('.board .board-list')
         evaluate_script("document.querySelectorAll('.board .board-list')[1].scrollTop = document.querySelectorAll('.board .board-list')[1].scrollHeight")
         wait_for_requests
 
@@ -449,11 +451,13 @@ describe 'Issue Boards', :js do
           expect(page).to have_selector('.card', count: 20)
           expect(page).to have_content('Showing 20 of 51 issues')
 
+          find('.board .board-list')
           evaluate_script("document.querySelectorAll('.board .board-list')[1].scrollTop = document.querySelectorAll('.board .board-list')[1].scrollHeight")
 
           expect(page).to have_selector('.card', count: 40)
           expect(page).to have_content('Showing 40 of 51 issues')
 
+          find('.board .board-list')
           evaluate_script("document.querySelectorAll('.board .board-list')[1].scrollTop = document.querySelectorAll('.board .board-list')[1].scrollHeight")
 
           expect(page).to have_selector('.card', count: 51)

spec/features/ci_lint_spec.rb

@@ -10,6 +10,7 @@ describe 'CI Lint', :js do
       visit ci_lint_path
       # Ace editor updates a hidden textarea and it happens asynchronously
       # `sleep 0.1` is actually needed here because of this
+      find('#ci-editor')
       execute_script("ace.edit('ci-editor').setValue(" + yaml_content.to_json + ");")
       sleep 0.1
       click_on 'Validate'

spec/features/dashboard/issues_spec.rb

@@ -87,8 +87,10 @@ RSpec.describe 'Dashboard Issues' do
       project_path = "/#{project.path_with_namespace}"
       project_json = { name: project.name_with_namespace, url: project_path }.to_json
 
-      # similate selection, and prevent overlap by dropdown menu
+      # simulate selection, and prevent overlap by dropdown menu
+      first('.project-item-select', visible: false)
       execute_script("$('.project-item-select').val('#{project_json}').trigger('change');")
+      find('#select2-drop-mask', visible: false)
       execute_script("$('#select2-drop-mask').remove();")
 
       find('.new-project-item-link').trigger('click')

spec/features/issues/markdown_toolbar_spec.rb

@@ -16,6 +16,7 @@ feature 'Issue markdown toolbar', :js do
     find('#note-body').native.send_key(:enter)
     find('#note-body').native.send_keys('bold')
 
+    find('.js-main-target-form #note-body')
     page.evaluate_script('document.querySelectorAll(".js-main-target-form #note-body")[0].setSelectionRange(4, 9)')
 
     first('.toolbar-btn').click
@@ -28,6 +29,7 @@ feature 'Issue markdown toolbar', :js do
     find('#note-body').native.send_key(:enter)
     find('#note-body').native.send_keys('underline')
 
+    find('.js-main-target-form #note-body')
     page.evaluate_script('document.querySelectorAll(".js-main-target-form #note-body")[0].setSelectionRange(4, 50)')
 
     find('.toolbar-btn:nth-child(2)').click

spec/features/merge_requests/conflicts_spec.rb

@@ -60,12 +60,14 @@ feature 'Merge request conflict resolution', :js do
       within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do
         click_button 'Edit inline'
         wait_for_requests
+        find('.files-wrapper .diff-file pre')
         execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");')
       end
 
       within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do
         click_button 'Edit inline'
         wait_for_requests
+        find('.files-wrapper .diff-file pre')
         execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).setValue("Gregor Samsa woke from troubled dreams");')
       end
 
@@ -139,6 +141,7 @@ feature 'Merge request conflict resolution', :js do
       it 'conflicts are resolved in Edit inline mode' do
         within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do
           wait_for_requests
+          find('.files-wrapper .diff-file pre')
           execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("Gregor Samsa woke from troubled dreams");')
         end
 

spec/features/merge_requests/edit_mr_spec.rb

@@ -66,6 +66,7 @@ feature 'Edit Merge Request' do
     end
 
     def get_textarea_height
+      find('#merge_request_description')
       page.evaluate_script('document.getElementById("merge_request_description").offsetHeight')
     end
   end

spec/features/merge_requests/mini_pipeline_graph_spec.rb

@@ -52,10 +52,12 @@ feature 'Mini Pipeline Graph', :js do
     end
 
     it 'should expand when hovered' do
+      find('.mini-pipeline-graph-dropdown-toggle')
       before_width = evaluate_script("$('.mini-pipeline-graph-dropdown-toggle:visible').outerWidth();")
 
       toggle.hover
 
+      find('.mini-pipeline-graph-dropdown-toggle')
       after_width = evaluate_script("$('.mini-pipeline-graph-dropdown-toggle:visible').outerWidth();")
 
       expect(before_width).to be < after_width

spec/features/projects/blobs/edit_spec.rb

@@ -20,6 +20,7 @@ feature 'Editing file blob', :js do
     def edit_and_commit
       wait_for_requests
       find('.js-edit-blob').click
+      find('#editor')
       execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")')
       click_button 'Commit changes'
     end

spec/features/projects/user_creates_files_spec.rb

@@ -60,6 +60,7 @@ describe 'User creates files' do
       end
 
       it 'creates and commit a new file', :js do
+        find('#editor')
         execute_script("ace.edit('editor').setValue('*.rbca')")
         fill_in(:file_name, with: 'not_a_file.md')
         fill_in(:commit_message, with: 'New commit message', visible: true)
@@ -75,6 +76,7 @@ describe 'User creates files' do
       end
 
       it 'creates and commit a new file with new lines at the end of file', :js do
+        find('#editor')
         execute_script('ace.edit("editor").setValue("Sample\n\n\n")')
         fill_in(:file_name, with: 'not_a_file.md')
         fill_in(:commit_message, with: 'New commit message', visible: true)
@@ -86,6 +88,7 @@ describe 'User creates files' do
 
         find('.js-edit-blob').click
 
+        find('#editor')
         expect(evaluate_script('ace.edit("editor").getValue()')).to eq("Sample\n\n\n")
       end
 
@@ -94,6 +97,7 @@ describe 'User creates files' do
 
         expect(page).to have_selector('.file-editor')
 
+        find('#editor')
         execute_script("ace.edit('editor').setValue('*.rbca')")
         fill_in(:commit_message, with: 'New commit message', visible: true)
         click_button('Commit changes')
@@ -108,6 +112,7 @@ describe 'User creates files' do
       it 'creates and commit a new file specifying a new branch', :js do
         expect(page).to have_selector('.file-editor')
 
+        find('#editor')
         execute_script("ace.edit('editor').setValue('*.rbca')")
         fill_in(:file_name, with: 'not_a_file.md')
         fill_in(:commit_message, with: 'New commit message', visible: true)
@@ -136,6 +141,7 @@ describe 'User creates files' do
 
         expect(page).to have_selector('.file-editor')
 
+        find('#editor')
         execute_script("ace.edit('editor').setValue('*.rbca')")
 
         fill_in(:file_name, with: 'not_a_file.md')

spec/features/projects/user_edits_files_spec.rb

@@ -23,6 +23,7 @@ describe 'User edits files' do
       find('.js-edit-blob').click
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
 
       expect(evaluate_script('ace.edit("editor").getValue()')).to eq('*.rbca')
@@ -40,6 +41,7 @@ describe 'User edits files' do
       find('.js-edit-blob').click
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
       fill_in(:commit_message, with: 'New commit message', visible: true)
       click_button('Commit changes')
@@ -57,6 +59,7 @@ describe 'User edits files' do
 
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
       fill_in(:commit_message, with: 'New commit message', visible: true)
       fill_in(:branch_name, with: 'new_branch_name', visible: true)
@@ -74,6 +77,7 @@ describe 'User edits files' do
       find('.js-edit-blob').click
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
       click_link('Preview changes')
 
@@ -103,6 +107,7 @@ describe 'User edits files' do
 
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
 
       expect(evaluate_script('ace.edit("editor").getValue()')).to eq('*.rbca')
@@ -119,6 +124,7 @@ describe 'User edits files' do
 
       find('.file-editor', match: :first)
 
+      find('#editor')
       execute_script("ace.edit('editor').setValue('*.rbca')")
       fill_in(:commit_message, with: 'New commit message', visible: true)
       click_button('Commit changes')
@@ -145,6 +151,7 @@ describe 'User edits files' do
         expect(page).not_to have_link('Fork')
         expect(page).not_to have_button('Cancel')
 
+        find('#editor')
         execute_script("ace.edit('editor').setValue('*.rbca')")
         fill_in(:commit_message, with: 'Another commit', visible: true)
         click_button('Commit changes')

spec/support/select2_helper.rb

@@ -16,6 +16,7 @@ module Select2Helper
 
     selector = options.fetch(:from)
 
+    first(selector, visible: false)
     if options[:multiple]
       execute_script("$('#{selector}').select2('val', ['#{value}']).trigger('change');")
     else