Merge pull request #2024 from WikiEducationFoundation/HeadlessChrome

Switch from Poltergeist to headless Chrome for Capybara feature specs

This changes the Capyabara driver from Poltergeist (based on the unmaintained PhantomJS headless browser) to Selenium and chromedriver. A few behavior bugs surfaced with Chrome that needed to be fixed in the frontend javascript, but most of the changes are just small adjustments to the specs. Chromedriver also helped pinpoint and fix some of causes of flapping tests, so I've mostly removed the 'pending' status from those tests and cleaned up the tests to pass consistently. Chrome renders CSS more accurately than poltergeist, so bugs and workarounds for misplaced elements could be removed.

Chromedriver doesn't support automatically failing tests when there are JavaScript errors, so a check for javascript errors after each feature spec had to be patched into rails_helper.

The one outstanding issue is that there are several javascript warnings that print to the console during certain tests. These come from `react-dom` code, but I haven't been able to pinpoint the cause.

.travis.yml

@@ -12,6 +12,7 @@ cache:
   - vendor
   - node_modules
 addons:
+  chrome: stable
   apt:
     packages:
     - pandoc

Gemfile

@@ -110,7 +110,6 @@ group :development, :test do
   gem 'rubocop-rspec-focused', require: false
   gem 'rubocop-rspec', require: false
   gem 'timecop' # Test utility for setting the time
-  gem 'poltergeist' # Capypara feature specs driven by PhantomJS
   gem 'factory_bot_rails' # Factory for creating ActiveRecord objects in tests
 end
 
@@ -119,6 +118,8 @@ group :test do
   gem 'rake', '>= 11.0'
   gem 'capybara'
   gem 'capybara-screenshot'
+  gem 'chromedriver-helper' # Capypara feature specs driven by headless Chrome
+  gem 'selenium-webdriver' # Capypara feature specs driven by headless Chrome
   gem 'database_cleaner'
   gem 'webmock'
   gem 'vcr' # Saves external web requests and replays them in tests

Gemfile.lock

@@ -92,6 +92,8 @@ GEM
     annotate (2.7.4)
       activerecord (>= 3.2, < 6.0)
       rake (>= 10.4, < 13.0)
+    archive-zip (0.11.0)
+      io-like (~> 0.3.0)
     arel (9.0.0)
     ast (2.4.0)
     bcrypt (3.1.12)
@@ -138,10 +140,14 @@ GEM
       capybara (>= 1.0, < 4)
       launchy
     chartkick (3.0.1)
+    childprocess (0.9.0)
+      ffi (~> 1.0, >= 1.0.11)
     choice (0.2.0)
+    chromedriver-helper (1.2.0)
+      archive-zip (~> 0.10)
+      nokogiri (~> 1.8)
     chronic (0.10.2)
     climate_control (0.2.0)
-    cliver (0.3.2)
     coderay (1.1.2)
     concurrent-ruby (1.0.5)
     connection_pool (2.2.2)
@@ -232,6 +238,7 @@ GEM
       concurrent-ruby (~> 1.0)
     i18n-js (3.0.11)
       i18n (>= 0.6.6, < 2)
+    io-like (0.3.0)
     jaro_winkler (1.5.1)
     jbuilder (2.7.0)
       activesupport (>= 4.2.0)
@@ -308,10 +315,6 @@ GEM
     parallel (1.12.1)
     parser (2.5.1.2)
       ast (~> 2.4.0)
-    poltergeist (1.18.1)
-      capybara (>= 2.1, < 4)
-      cliver (~> 0.3.1)
-      websocket-driver (>= 0.2.0)
     powerpack (0.1.2)
     premailer (1.11.1)
       addressable
@@ -428,7 +431,11 @@ GEM
     ruby_dep (1.5.0)
     ruby_parser (3.11.0)
       sexp_processor (~> 4.9)
+    rubyzip (1.2.1)
     safe_yaml (1.0.4)
+    selenium-webdriver (3.11.0)
+      childprocess (~> 0.5)
+      rubyzip (~> 1.2)
     sentimental (1.4.1)
     sentry-raven (2.7.4)
       faraday (>= 0.7.6, < 1.0)
@@ -523,6 +530,7 @@ DEPENDENCIES
   capybara
   capybara-screenshot
   chartkick
+  chromedriver-helper
   connection_pool
   dalli
   database_cleaner
@@ -550,7 +558,6 @@ DEPENDENCIES
   paper_trail
   paperclip
   piwik_analytics!
-  poltergeist
   premailer-rails
   pry-rails
   puma
@@ -570,6 +577,7 @@ DEPENDENCIES
   rubocop
   rubocop-rspec
   rubocop-rspec-focused
+  selenium-webdriver
   sentimental
   sentry-raven
   shoulda-matchers

app/assets/javascripts/components/categories/add_category_button.jsx

@@ -7,7 +7,7 @@ import Select from 'react-select';
 import { initiateConfirm } from '../../actions/confirm_actions';
 import TextInput from '../common/text_input';
 import Popover from '../common/popover.jsx';
-import PopoverExpandable from '../high_order/popover_expandable.jsx';
+import Expandable from '../high_order/expandable.jsx';
 import CourseUtils from '../../utils/course_utils.js';
 
 const AddCategoryButton = createReactClass({
@@ -210,5 +210,5 @@ const AddCategoryButton = createReactClass({
 const mapDispatchToProps = { initiateConfirm };
 
 export default connect(null, mapDispatchToProps)(
-  PopoverExpandable(AddCategoryButton)
+  Expandable(AddCategoryButton)
 );

app/assets/javascripts/components/overview/tag_editable.jsx

@@ -79,37 +79,34 @@ const TagEditable = createReactClass({
       );
     });
 
-    let tagSelect;
-    if (this.props.availableTags.length > 0) {
-      const availableOptions = this.props.availableTags.map(tag => {
-        return { label: tag, value: tag };
-      });
-      const tagOptions = [...this.state.createdTagOption, ...availableOptions];
-      let addTagButtonDisabled = true;
-      if (this.state.selectedTag) {
-        addTagButtonDisabled = false;
-      }
-      tagSelect = (
-        <tr>
-          <th>
-            <div className="select-with-button">
-              <Select.Creatable
-                className="fixed-width"
-                ref="tagSelect"
-                name="tag"
-                value={this.state.selectedTag}
-                placeholder={I18n.t('courses.tag_select')}
-                onChange={this.handleChangeTag}
-                options={tagOptions}
-              />
-              <button type="submit" className="button dark" disabled={addTagButtonDisabled} onClick={this.addTag}>
-                Add
-              </button>
-            </div>
-          </th>
-        </tr>
-      );
+    const availableOptions = this.props.availableTags.map(tag => {
+      return { label: tag, value: tag };
+    });
+    const tagOptions = [...this.state.createdTagOption, ...availableOptions];
+    let addTagButtonDisabled = true;
+    if (this.state.selectedTag) {
+      addTagButtonDisabled = false;
     }
+    const tagSelect = (
+      <tr>
+        <th>
+          <div className="select-with-button">
+            <Select.Creatable
+              className="fixed-width"
+              ref="tagSelect"
+              name="tag"
+              value={this.state.selectedTag}
+              placeholder={I18n.t('courses.tag_select')}
+              onChange={this.handleChangeTag}
+              options={tagOptions}
+            />
+            <button type="submit" className="button dark" disabled={addTagButtonDisabled} onClick={this.addTag}>
+              Add
+            </button>
+          </div>
+        </th>
+      </tr>
+    );
 
     return (
       <div key="tags" className="pop__container tags open" onClick={this.stop}>
@@ -122,7 +119,6 @@ const TagEditable = createReactClass({
       </div>
     );
   }
-
 });
 
 const mapStateToProps = state => ({

app/assets/javascripts/utils/api.js

@@ -416,7 +416,6 @@ slide_id=${opts.slide_id}`,
       .fail(function (obj, status) {
         this.obj = obj;
         this.status = status;
-        console.error('Couldn\'t save course!');
         RavenLogger.obj = this.obj;
         RavenLogger.status = this.status;
         Raven.captureMessage('saveCourse failed', {

spec/features/admin_role_spec.rb

@@ -5,7 +5,6 @@
 describe 'Admin users', type: :feature, js: true do
   before do
     page.current_window.resize_to(1920, 1080)
-    page.driver.browser.url_blacklist = ['https://wikiedu.org']
   end
 
   before do
@@ -62,16 +61,13 @@
   describe 'visiting the dashboard' do
     it 'sees submitted courses awaiting approval' do
       visit root_path
-      sleep 1
       expect(page).to have_content 'Submitted & Pending Approval'
       expect(page).to have_content 'My Submitted Course'
     end
   end
 
   describe 'adding a course to a campaign' do
     it 'makes the course live' do
-      pending 'This sometimes fails on travis.'
-
       stub_oauth_edit
       stub_chat_channel_create_success
 
@@ -82,14 +78,12 @@
       click_button 'Edit Details'
       within '#course_campaigns' do
         page.find('.button.border.plus').click
-        find('div.Select').send_keys('Fall 2015', :enter)
-        omniclick find('.pop button', visible: true)
+        find('input').send_keys('Fall 2015', :enter)
+        click_button 'Add'
       end
 
       expect(page).to have_content 'Your course has been published'
       expect(page).not_to have_content 'This course has been submitted for approval by its creator'
-
-      pass_pending_spec
     end
   end
 
@@ -117,15 +111,13 @@
 
   describe 'adding a tag to a course' do
     it 'works' do
-      pending 'This sometimes fails on travis.'
-
       stub_token_request
       visit "/courses/#{Course.first.slug}"
       click_button('Edit Details')
-      within '.tags' do
-        page.find('.button.border.plus').click
-        page.find('input').set 'My Tag'
-        find('.pop button', visible: true).click
+      within '.pop__container.tags' do
+        click_button '+'
+        find('input').send_keys('My Tag', :enter)
+        click_button 'Add'
       end
 
       sleep 1
@@ -134,28 +126,22 @@
 
       # Add the same tag again
       click_button('Edit Details')
-      within('div.tags') do
-        page.find('.button.border.plus').click
-      end
-      page.find('section.overview input[placeholder="Tag"]').set 'My Tag'
-      page.all('.pop button', visible: true)[1].click
+      within '.pop__container.tags' do
+        click_button '+'
+        find('input').send_keys('My Tag', :enter)
+        click_button 'Add'
 
-      # Delete the tag
-      within('div.tags') do
+        # Delete the tag
         click_button '-'
       end
       sleep 1
       visit "/courses/#{Course.first.slug}"
       expect(page).not_to have_content 'My Tag'
-
-      pass_pending_spec
     end
   end
 
   describe 'linking a course to its Salesforce record' do
     it 'makes the Link to Salesforce button appear' do
-      pending 'This sometimes fails on travis.'
-
       stub_token_request
       expect_any_instance_of(Restforce::Data::Client).to receive(:update!).and_return(true)
 
@@ -165,8 +151,6 @@
       end
       expect(page).to have_content 'Open in Salesforce'
       expect(Course.first.flags[:salesforce_id]).to eq('a0f1a011101Xyas')
-
-      pass_pending_spec
     end
   end
 end

spec/features/campaign_overview_spec.rb

@@ -90,7 +90,7 @@ module ResetLocale
       # Number of students
       # one non-instructor student per course and one instructor-student per course
       student_count = campaign_course_count * 2
-      stat_text = "#{student_count} \n#{I18n.t('courses.students')}"
+      stat_text = "#{student_count}\n#{I18n.t('courses.students')}"
       expect(page.find('.stat-display')).to have_content stat_text
 
       # Words added
@@ -114,7 +114,7 @@ module ResetLocale
 
       it 'falls back when locale is not available' do
         visit "/campaigns/#{campaign.slug}/overview?locale=aa"
-        expect(page.find('.stat-display')).to have_content "20 \nStudents"
+        expect(page.find('.stat-display')).to have_content "20\nStudents"
       end
 
       # TODO: Test somewhere that has access to the request.
@@ -196,8 +196,8 @@ module ResetLocale
         it 'shows an error for invalid dates' do
           find('.campaign-details .rails_editable-edit').click
           find('#use_dates').click
-          fill_in('campaign_start', with: '2016-01-10')
-          fill_in('campaign_end', with: 'Invalid date')
+          fill_in('campaign_start', with: '2016-01-10'.to_date)
+          # fill_in('campaign_end', with: 'Invalid date')
           find('.campaign-details .rails_editable-save').click
           expect(page).to have_content(I18n.t('error.invalid_date', key: 'End'))
           find('#campaign_end', visible: true) # field with the error should be visible
@@ -206,8 +206,8 @@ module ResetLocale
         it 'updates the date fields properly, and unsets if #use_dates is unchecked' do
           find('.campaign-details .rails_editable-edit').click
           find('#use_dates').click
-          fill_in('campaign_start', with: '2016-01-10')
-          fill_in('campaign_end', with: '2016-02-10')
+          fill_in('campaign_start', with: '2016-01-10'.to_date)
+          fill_in('campaign_end', with: '2016-02-10'.to_date)
           find('.campaign-details .rails_editable-save').click
           expect(campaign.reload.start).to eq(Time.new(2016, 1, 10, 0, 0, 0, 0))
           expect(campaign.end).to eq(Time.new(2016, 2, 10, 23, 59, 59, 0))
@@ -235,9 +235,7 @@ module ResetLocale
       it 'throws an error if you enter the wrong campaign title when trying to delete it' do
         wrong_title = 'Not the title of the campaign'
         accept_alert(with: /"#{wrong_title}"/) do
-          accept_prompt(with: wrong_title) do
-            find('.campaign-delete .button').click
-          end
+          find('.campaign-delete .button').click
         end
       end
     end

spec/features/campaigns_spec.rb

@@ -56,7 +56,7 @@
       find('.create-campaign-button').click
       fill_in('campaign_title', with: 'My Campaign Test')
       find('#use_dates').click
-      fill_in('campaign_start', with: '2016-01-10') # end date not supplied
+      fill_in('campaign_start', with: '2016-01-10'.to_date) # end date not supplied
       find('.wizard__form .button__submit').click
       find('.wizard__panel', visible: true)
       expect(page).to have_content(I18n.t('error.invalid_date', key: 'End'))
@@ -69,8 +69,8 @@
       fill_in('campaign_title', with: title)
       fill_in('campaign_description', with: description)
       find('#use_dates').click
-      fill_in('campaign_start', with: '2016-01-10')
-      fill_in('campaign_end', with: '2016-02-10')
+      fill_in('campaign_start', with: '2016-01-10'.to_date)
+      fill_in('campaign_end', with: '2016-02-10'.to_date)
       find('.wizard__form .button__submit').click
       expect(Campaign.last.title).to eq(title)
       expect(Campaign.last.description).to eq(description)