address a number of issues with the js test suite

This patch handles a couple of things: - `require` calls should not be used at the start of test files, but they were. This switches them to `define`. - There were a handful of tests that were not fulfilling initialized promises, which led to the test suite hanging. - There was a failure that popped up sometimes because `helpDialog` was not cleaning up after itself. This adds a protection to make sure that the instance is initialized before it is used. Change-Id: Ie57f5ee6d45216f0d4071d34cc476201c3b7c9f6 Reviewed-on: https://gerrit.instructure.com/48447 Tested-by: Jenkins Reviewed-by: Simon Williams <[email protected]> QA-Review: Adam Stone <[email protected]> Product-Review: Simon Williams <[email protected]>
ralacas · Feb 20, 2015 · ccb3db7 · ccb3db7
1 parent 3308ba8
commit ccb3db7
Show file tree

Hide file tree

Showing 33 changed files with 97 additions and 76 deletions.
diff --git a/app/coffeescripts/helpDialog.coffee b/app/coffeescripts/helpDialog.coffee
@@ -76,6 +76,7 @@ define [
       }, {
         step: =>
           #reposition vertically to reflect current height
+          @initDialog() unless @dialogInited
           @$dialog?.dialog('option', 'position', 'center')
         duration: 100
         complete: ->

diff --git a/app/coffeescripts/models/WikiPageRevision.coffee b/app/coffeescripts/models/WikiPageRevision.coffee
@@ -7,6 +7,7 @@ define [
   'compiled/str/splitAssetString'
   'compiled/util/PandaPubPoller'
   'compiled/jquery.rails_flash_notifications'
+  'jquery.disableWhileLoading'
 ], ($, _, Backbone, I18n, DefaultUrlMixin, splitAssetString, PandaPubPoller) ->
 
   pageRevisionOptions = ['contextAssetString', 'page', 'pageUrl', 'latest', 'summary']

diff --git a/app/coffeescripts/react_files/components/FilesUsage.coffee b/app/coffeescripts/react_files/components/FilesUsage.coffee
@@ -10,13 +10,15 @@ define [
 
   FilesUsage = React.createClass
     displayName: 'FilesUsage'
+    url: ->
+      "/api/v1/#{@props.contextType}/#{@props.contextId}/files/quota"
 
     propTypes:
       contextType: customPropTypes.contextType.isRequired
       contextId: customPropTypes.contextId.isRequired
 
     update: ->
-      $.get "/api/v1/#{@props.contextType}/#{@props.contextId}/files/quota", (data) =>
+      $.get @url(), (data) =>
         @setState(data)
 
     componentDidMount: ->

diff --git a/app/coffeescripts/views/wiki/WikiPageRevisionView.coffee b/app/coffeescripts/views/wiki/WikiPageRevisionView.coffee
@@ -34,10 +34,13 @@ define [
       json.edited_by = json.edited_by?.display_name
       json
 
+    windowLocation: ->
+      return window.location;
+
     restore: (ev) ->
       ev?.preventDefault()
       @model.restore().done (attrs) =>
         if @pages_path
-          window.location.href = "#{@pages_path}/#{attrs.url}/revisions"
+          @windowLocation().href = "#{@pages_path}/#{attrs.url}/revisions"
         else
-          window.location.reload()
+          @windowLocation().reload()
diff --git a/app/jsx/external_apps/lib/ExternalAppsStore.jsx b/app/jsx/external_apps/lib/ExternalAppsStore.jsx
@@ -222,4 +222,4 @@ define([
   };
 
   return store;
-});
+});
diff --git a/karma.conf.js b/karma.conf.js
@@ -25,15 +25,15 @@ module.exports = function(config) {
 
     exclude: [],
 
-    // 'dots', 'progress', 'junit', 'growl', 'coverage'
-    reporters: ['progress'], 
+    // 'dots', 'progress', 'junit', 'growl', 'coverage', 'spec'
+    reporters: ['progress'],
 
     port: 9876,
 
     colors: true,
 
     // config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG
-    logLevel: config.LOG_INFO, 
+    logLevel: config.LOG_INFO,
 
     autoWatch: true,
 

diff --git a/spec/coffeescripts/backbone-ext/Model/dateAttributesSpec.coffee b/spec/coffeescripts/backbone-ext/Model/dateAttributesSpec.coffee
@@ -1,4 +1,4 @@
-require ['compiled/backbone-ext/Model'], (Model) ->
+define ['compiled/backbone-ext/Model'], (Model) ->
 
   module 'dateAttributes'
 

diff --git a/spec/coffeescripts/behaviors/autocompleteSpec.coffee b/spec/coffeescripts/behaviors/autocompleteSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'underscore'
   'compiled/behaviors/autocomplete'

diff --git a/spec/coffeescripts/behaviors/elementTogglerSpec.coffee b/spec/coffeescripts/behaviors/elementTogglerSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'compiled/behaviors/elementToggler'
 ], ($, elementToggler)->

diff --git a/spec/coffeescripts/calendar/AgendaViewSpec.coffee b/spec/coffeescripts/calendar/AgendaViewSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'underscore'
   'timezone'

diff --git a/spec/coffeescripts/calendar/TimeBlockListSpec.coffee b/spec/coffeescripts/calendar/TimeBlockListSpec.coffee
@@ -1,5 +1,5 @@
 # requires jquery and date.js
-require [
+define [
   'jquery'
   'compiled/calendar/TimeBlockList'
 ], ($, TimeBlockList) ->

diff --git a/spec/coffeescripts/calendar/TimeBlockRowSpec.coffee b/spec/coffeescripts/calendar/TimeBlockRowSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'compiled/calendar/TimeBlockList'
   'compiled/calendar/TimeBlockRow'

diff --git a/spec/coffeescripts/collections/CollectionSpec.coffee b/spec/coffeescripts/collections/CollectionSpec.coffee
@@ -19,8 +19,10 @@ define [
 
   module 'Backbone.Collection',
     setup: ->
+      @xhr = sinon.useFakeXMLHttpRequest()
       @ajaxSpy = sinon.spy $, 'ajax'
     teardown: ->
+      @xhr.restore
       $.ajax.restore()
 
   test 'default options', ->

diff --git a/spec/coffeescripts/flashNotificationSpec.coffee b/spec/coffeescripts/flashNotificationSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'compiled/jquery.rails_flash_notifications'
 ], ($) ->
@@ -23,4 +23,4 @@ require [
 
   test 'screenreader message', ->
     $.screenReaderFlashMessage('<script>evil()</script>')
-    ok $('#flash_screenreader_holder span').html().match(/&lt;script&gt;/)
+    ok $('#flash_screenreader_holder span').html().match(/&lt;script&gt;/)
diff --git a/spec/coffeescripts/helpDialogSpec.coffee b/spec/coffeescripts/helpDialogSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'compiled/helpDialog'
   'helpers/fakeENV'

diff --git a/spec/coffeescripts/helpers/ajax_mocks/api/v1/courses.coffee b/spec/coffeescripts/helpers/ajax_mocks/api/v1/courses.coffee
@@ -1,4 +1,4 @@
-require ['support/jquery.mockjax'], ($) ->
+define ['support/jquery.mockjax'], ($) ->
   $.mockjax
     url:  /\/api\/v1\/courses\/\d+(\?.+)?$/
     responseText: [

diff --git a/spec/coffeescripts/helpers/ajax_mocks/help_links.coffee b/spec/coffeescripts/helpers/ajax_mocks/help_links.coffee
@@ -1,4 +1,4 @@
-require [ 'support/jquery.mockjax'], ($) ->
+define [ 'support/jquery.mockjax'], ($) ->
   $.mockjax
     url:  '/help_links*'
     responseText: [{
@@ -12,4 +12,4 @@ require [ 'support/jquery.mockjax'], ($) ->
       url: "http://help.instructure.com/forums/337215-feature-requests"
       text: "Request a Feature"
       available_to: [ "user", "student", "teacher", "admin" ]
-    }]
+    }]
diff --git a/spec/coffeescripts/jquery/mediaCommentSpec.coffee b/spec/coffeescripts/jquery/mediaCommentSpec.coffee
@@ -1,16 +1,16 @@
-require [
+define [
   'jquery'
   'compiled/jquery/mediaComment'
 ], ($)->
 
   module 'mediaComment',
-    setup: -> 
+    setup: ->
       @server = sinon.fakeServer.create()
       window.INST.kalturaSettings = "settings set" # pretend kalturaSettings are set.
       @$holder = $('<div id="media-holder">').appendTo('#fixtures')
 
-    teardown: -> 
-      window.INST.kalturaSettings = null 
+    teardown: ->
+      window.INST.kalturaSettings = null
       @server.restore()
       @$holder.remove()
 
@@ -28,7 +28,7 @@ require [
       JSON.stringify(resp)
     ]
 
-  test "video player is displayed inline", -> 
+  test "video player is displayed inline", ->
     id = 10 #ID doesn't matter since we mock out the server
     @$holder.mediaComment('show_inline', id)
     mockServerResponse(@server, id)
@@ -45,7 +45,7 @@ require [
     equal @$holder.find('video').length, 0, 'There should not be a video tag'
 
 
-  test "video player includes url sources provided by the server", -> 
+  test "video player includes url sources provided by the server", ->
     id = 10 #ID doesn't matter since we mock out the server
     @$holder.mediaComment('show_inline', id)
     mockServerResponse(@server, id)

diff --git a/spec/coffeescripts/jquery/mediaCommentThumbnailSpec.coffee b/spec/coffeescripts/jquery/mediaCommentThumbnailSpec.coffee
@@ -1,4 +1,4 @@
-require [
+define [
   'jquery'
   'underscore'
   'compiled/jquery/mediaCommentThumbnail'

diff --git a/spec/coffeescripts/jsx/external_apps/lib/ExternalAppsStoreSpec.coffee b/spec/coffeescripts/jsx/external_apps/lib/ExternalAppsStoreSpec.coffee
@@ -1,6 +1,12 @@
-define ['jsx/external_apps/lib/ExternalAppsStore'], (store) ->
+define [
+  'jsx/external_apps/lib/ExternalAppsStore',
+  'helpers/fakeENV'
+], (store, fakeENV) ->
   module 'ExternalApps.ExternalAppsStore',
     setup: ->
+      fakeENV.setup({
+        CONTEXT_BASE_URL: "/courses/1"
+      })
       @server = sinon.fakeServer.create()
       store.reset()
       @tools = [

diff --git a/spec/coffeescripts/models/FileMigrationSpec.coffee b/spec/coffeescripts/models/FileMigrationSpec.coffee
@@ -1,6 +1,6 @@
-require [
+define [
   'Backbone'
-], (Backbone) -> 
-  module 'FileMigrationSpec', 
-    setup: -> 
-    teardown: -> 
+], (Backbone) ->
+  module 'FileMigrationSpec',
+    setup: ->
+    teardown: ->
diff --git a/spec/coffeescripts/models/WikiPageRevisionSpec.coffee b/spec/coffeescripts/models/WikiPageRevisionSpec.coffee
@@ -3,6 +3,7 @@ define [
   'underscore'
   'compiled/models/WikiPage'
   'compiled/models/WikiPageRevision'
+  'jquery.ajaxJSON'
 ], ($, _, WikiPage, WikiPageRevision) ->
 
   module 'WikiPageRevision::urls'
@@ -43,10 +44,9 @@ define [
   test 'restore POSTs to the revision', ->
     revision = new WikiPageRevision {revision_id: 42}, contextAssetString: 'course_73', pageUrl: 'page-url'
     mock = @mock($)
-    mock.expects('ajaxJSON').atLeast(1).withArgs('/api/v1/courses/73/pages/page-url/revisions/42', 'POST').returns($.Deferred())
+    mock.expects('ajaxJSON').atLeast(1).withArgs('/api/v1/courses/73/pages/page-url/revisions/42', 'POST').returns($.Deferred().resolve())
     revision.restore()
 
-
   module 'WikiPageRevision::fetch'
   test 'the summary flag is passed to the server', ->
     @stub($, 'ajax').returns($.Deferred())

diff --git a/spec/coffeescripts/object/unflattenSpec.coffee b/spec/coffeescripts/object/unflattenSpec.coffee
@@ -1,15 +1,15 @@
-require ['compiled/object/unflatten'], (unflatten) ->
+define ['compiled/object/unflatten'], (unflatten) ->
 
   module 'unflatten'
 
   test 'simple object', ->
-    input = 
+    input =
       foo: 1
       bar: 'baz'
     deepEqual unflatten(input), input
-  
+
   test 'nested params', ->
-    input = 
+    input =
       'a[0]': 1
       'a[1]' : 2
       'a[2]' : 3
@@ -20,7 +20,7 @@ require ['compiled/object/unflatten'], (unflatten) ->
       'c[g]' : false
       'c[h]' : ''
       'i': 7
-    
+
     expected =
       a: [ 1, 2, 3 ]
       b: 4
@@ -32,5 +32,5 @@ require ['compiled/object/unflatten'], (unflatten) ->
         g: false
         h: ''
       i: 7
-    
+
     deepEqual unflatten(input), expected
diff --git a/spec/coffeescripts/react_files/components/FilesUsageSpec.coffee b/spec/coffeescripts/react_files/components/FilesUsageSpec.coffee
@@ -4,49 +4,49 @@ define [
   'jquery'
   'compiled/react_files/components/FilesUsage'
 ], (React, $, FilesUsage) ->
-  Simulate = React.addons.TestUtils.Simulate
+  TestUtils = React.addons.TestUtils
+  Simulate = TestUtils.Simulate
 
   filesUpdateTest = (props, test) ->
-    @filesUsage = React.renderComponent(FilesUsage(props), $('<div>').appendTo('body')[0])
+    @server = sinon.fakeServer.create()
+    @filesUsage = TestUtils.renderIntoDocument(FilesUsage(props))
 
     test()
 
     React.unmountComponentAtNode(@filesUsage.getDOMNode().parentNode)
 
+    @server.restore()
+
   module 'FilesUsage#update',
   test "makes a get request with contextType and contextId", ->
     sinon.stub($, 'get')
-    filesUpdateTest {contextType: 5, contextId: 4}, ->
+    filesUpdateTest {contextType: 'users', contextId: 4}, ->
        @filesUsage.update()
-       ok $.get.calledWith("/api/v1/5/4/files/quota"), "makes get request with correct params"
+       ok $.get.calledWith(@filesUsage.url()), "makes get request with correct params"
     $.get.restore()
 
   test "sets state with ajax requests returned data", ->
     data = {foo: 'bar'}
-    server = sinon.fakeServer.create()
 
-    server.respondWith "/api/v1/5/4/files/quota", [
-      200
-      'Content-Type': 'application/json'
-      JSON.stringify data
-    ]
+    filesUpdateTest {contextType: 'users', contextId: 4}, ->
+      @server.respondWith @filesUsage.url(), [
+        200
+        'Content-Type': 'application/json'
+        JSON.stringify data
+      ]
 
-    filesUpdateTest {contextType: 5, contextId: 4}, ->
       sinon.spy(@filesUsage, 'setState')
 
       @filesUsage.update()
-      server.respond()
+      @server.respond()
 
       ok @filesUsage.setState.calledWith(data), 'called set state with returned get request data'
 
       @filesUsage.setState.restore()
 
-    server.restore()
-
   test 'update called after component mounted', ->
-
-    filesUpdateTest {contextType: 5, contextId: 4}, ->
-      sinon.stub(@filesUsage, 'update')
+    filesUpdateTest {contextType: 'users', contextId: 4}, ->
+      sinon.stub(@filesUsage, 'update').returns(true)
       @filesUsage.componentDidMount()
       ok @filesUsage.update.calledOnce, "called update after it mounted"
       @filesUsage.update.restore()

diff --git a/spec/coffeescripts/userSettingsSpec.coffee b/spec/coffeescripts/userSettingsSpec.coffee
@@ -1,5 +1,5 @@
 
-require [
+define [
   'compiled/userSettings'
 ], (userSettings)->
 
@@ -46,4 +46,4 @@ require [
     equal userSettings.contextGet('foo'), 2
 
     ENV.context_asset_string = 'course_1'
-    equal userSettings.contextGet('foo'), 1
+    equal userSettings.contextGet('foo'), 1
diff --git a/spec/coffeescripts/util/deparamSpec.coffee b/spec/coffeescripts/util/deparamSpec.coffee
@@ -1,11 +1,11 @@
-require ['compiled/util/deparam'], (deparam) ->
-  
+define ['compiled/util/deparam'], (deparam) ->
+
   params_str = 'a[0]=4&a[1]=5&a[2]=6&b[x][]=7&b[y]=8&b[z][0]=9&b[z][1]=0&b[z][2]=true&b[z][3]=false&b[z][4]=undefined&b[z][5]=&c=1'
   params_obj = { a:['4','5','6'], b:{x:['7'], y:'8', z:['9','0','true','false','undefined','']}, c:'1' }
   params_obj_coerce = { a:[4,5,6], b:{x:[7], y:8, z:[9,0,true,false,undefined,'']}, c:1 }
-  
+
   module "deparam"
-  
+
   test "deparam", ->
     deepEqual deparam(), {}, "deparam()"
     deepEqual deparam(params_str), params_obj, "deparam( String )"
-Original file line number
+Diff line change
@@ Expand Up / @@ -222,4 +222,4 @@ define([ @@
       };
       return store;
-    });
+    });