Page MenuHomePhabricator

[Spike, 8hrs] [Bug] Skin.test.js should call jQuery.tearDown()
Closed, ResolvedPublic0 Story Points

Description

We are having problems with our unit tests in Minerva. This task (currently a spike) encompasses the work to identify the problem, propose a solution and then estimate and fix the bug.

Skin.test.js sets up jQuery as a dependency but doesn't tear it down in QUnit.afterEach(). However, when jQuery.tearDown() is called, several headless tests fail:

not ok 66 MobileFrontend mobile.startup/View > View
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:22:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at View.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at new View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:23:13)
  ...
not ok 67 MobileFrontend mobile.startup/View > View, jQuery proxy functions
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:30:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at View.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at new View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:31:13)
  ...
not ok 68 MobileFrontend mobile.startup/View > View#preRender
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:53:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at ChildView.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at ChildView.View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at new ChildView (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:56:8)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:66:9)
  ...
not ok 69 MobileFrontend mobile.startup/View > View#postRender
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:70:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at ChildView.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at ChildView.View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at new ChildView (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:73:8)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:83:2)
  ...
not ok 70 MobileFrontend mobile.startup/View > View#delegateEvents
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:87:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at EventsView.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at EventsView.View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at new EventsView (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:91:8)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:114:9)
  ...
not ok 71 MobileFrontend mobile.startup/View > View#render (with isTemplateMode)
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:127:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): jQuery requires a window with a document"
  severity: failed
  actual: null
  expected: undefined
  stack: Error: jQuery requires a window with a document
    at module.exports (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/node_modules/jquery/dist/jquery.js:31:12)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:129:13)
  ...
not ok 72 MobileFrontend mobile.startup/View > View#render events (with isTemplateMode)
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:165:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at TemplateModeView.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at TemplateModeView.View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at new TemplateModeView (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:168:8)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:182:9)
  ...
not ok 73 MobileFrontend mobile.startup/View > View with className option
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:198:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17): Cannot read property 'apply' of undefined"
  severity: failed
  actual: null
  expected: undefined
  stack: TypeError: Cannot read property 'apply' of undefined
    at Object.extend (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/util.js:113:18)
    at View.initialize (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:167:18)
    at new View (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/src/mobile.startup/View.js:89:18)
    at Object.<anonymous> (/home/stephen/Code/wmf/boxwiki/core/extensions/MobileFrontend/tests/node-qunit/mobile.startup/View.test.js:209:4)
  ...

This may be an issue with how View and OO are configured. OO is set up / torn down as needed but View is required just once and makes use of the global OO.mixinClass(), and all View subclasses are required once and call mfExtend() which also makes calls to OO methods.

Related patchsets:

Spike questions to answer

Questions should be answered in the comments of this task:

  • What is causing this problem?
  • What are the option(s) for fixing the problem?

Sign off steps

  • Repurpose this spike as a task updating the task with the answered questions
  • New task should be discussed and estimated

Event Timeline

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptNov 27 2018, 4:19 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Niedzielski updated the task description. (Show Details)Nov 27 2018, 4:43 PM
Jdlrobson renamed this task from [Bug] Skin.test.js should call jQuery.tearDown() to [Spike, 8hrs][Bug] Skin.test.js should call jQuery.tearDown().Dec 11 2018, 8:15 PM
Jdlrobson added a project: Spike.
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 0.
Jdlrobson added a subscriber: Jdlrobson.

We've repurposed this as a spike to work out the problem here. Once we know that, we'll pull the task out of the sprint as a repurposed task and estimate and fix it.

I see in the description that only the View tests are failing. Another avenue of exploration might be in the View.test.js file itself. I noticed that in that file jQuery is set up without JSDom, which could also be an issue.

MBinder_WMF renamed this task from [Spike, 8hrs][Bug] Skin.test.js should call jQuery.tearDown() to [Spike, 8hrs] [Bug] Skin.test.js should call jQuery.tearDown().Dec 12 2018, 6:25 PM

Change 479593 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Ensure View.test.js sets up JSDom before jQuery

https://gerrit.wikimedia.org/r/479593

This is second time I've attached a task to a spike, bad me! (but I didn't want this failing View.test.js to block other patches like T210208).

The patch adds the teadown in Skin.test.js and fixed View.test.js by adding the JSDom setup to that test. JQuery was failing because of no global window object. More specifically, the lack of a window object causes jQuery to export a factory function in Node instead of the proper jQuery class.

Ok I'm backtracking here, can't handle this kanbanana chaos (deep breath.. starting over).

I've identified the root cause of the failing tests (comment above), and created a new task T211963 to reflect that work (I'm not a fan of repurposing tasks).

I'm taking this out of code-review and putting it in sign-off, since I think my comment above, and the new task (and patch attached to it) answers the question in this spike.

Jdlrobson assigned this task to nray.Dec 17 2018, 6:05 PM
nray added a comment.Dec 17 2018, 11:18 PM

This is second time I've attached a task to a spike, bad me! (but I didn't want this failing View.test.js to block other patches like T210208).
The patch adds the teadown in Skin.test.js and fixed View.test.js by adding the JSDom setup to that test. JQuery was failing because of no global window object. More specifically, the lack of a window object causes jQuery to export a factory function in Node instead of the proper jQuery class.

As seen here https://github.com/jquery/jquery/blob/e743cbd28553267f955f71ea7248377915613fd9/src/wrapper.js#L19-L38

nray closed this task as Resolved.Dec 17 2018, 11:32 PM
nray updated the task description. (Show Details)

Looks like we are done here. Reviewed the rest of our node qunit tests to ensure that the files that are using jquery also call its teardown method in the afterEach hook.

nray removed nray as the assignee of this task.Dec 17 2018, 11:40 PM
nray added a subscriber: nray.