Page MenuHomePhabricator

Tests broken under sinon 4.x
Closed, ResolvedPublic3 Estimated Story Points

Description

We are using sinon v2.x through mw-node-qunit. I've updated the major version of mw-node-qunit to 3 to update sinon to the latest version, 4.x.

Sinon has upgraded major version with breaking changes in July, so our tests are not compatible with the library now and going forward unless we update them.

See migration guides and changelog:

Here is a test run with the failures:

$ npm i mw-node-qunit@latest
$ npm ls mw-node-qunit
└── mw-node-qunit@3.0.0
$ npm run test:node

> @ test:node /Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popu
ps
> node tests/node-qunit/run.js 'tests/node-qunit/**/*.test.js' | tap-dot


  .xxxxxxxxxxxxxxxxxxx.xxx.......................xxxx..........................
...............................................................................
...............................................................................
...............................................................................
...............................................................................
.......................................................................x


  387 tests
  438 passed
  27 failed

  Failed Tests:   There were 27 failures

    x beforeEach failed on #linkDwell: stub(obj, 'meth', fn) has been removed,see documentation, expected: undefined, got: null, test: #linkDwell, module: ext.popups/actions#linkDwell @integration, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:116:7)
    x beforeEach failed on #linkDwell doesn't continue when previews are disabled: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: #linkDwell doesn't continue when previews are disabled, module: ext.popups/actions#linkDwell @integration, source: TypeError: stub(obj,'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:166:7)
    x beforeEach failed on #linkDwell doesn't continue if the token has changed: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: #linkDwell doesn't continue if the token has changed, module: ext.popups/actions#linkDwell @integration, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:194:7)
    x beforeEach failed on #linkDwell dispatches the fetch action: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null,test: #linkDwell dispatches the fetch action, module: ext.popups/actions#linkDwell @integration, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:232:7)
    x beforeEach failed on it should fetch data from the gateway immediately: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined,got: null, test: it should fetch data from the gateway immediately, module: ext.popups/actions#fetch, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:287:7)
    x beforeEach failed on it should dispatch the FETCH_END action when the API request ends: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it should dispatch the FETCH_END action when the API request ends, module: ext.popups/actions#fetch, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:306:7)
    x beforeEach failed on it should delay dispatching the FETCH_COMPLETE action: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it should delay dispatching the FETCH_COMPLETE action, module: ext.popups/actions#fetch, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:326:7)
    x beforeEach failed on it should dispatch the FETCH_FAILED action when therequest fails: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it should dispatch the FETCH_FAILED action when the request fails, module: ext.popups/actions#fetch, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:378:7)
    x beforeEach failed on it should dispatch the FETCH_FAILED action when therequest fails even after the wait timeout: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it should dispatch the FETCH_FAILED action when the request fails even after the wait timeout, module: ext.popups/actions#fetch, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:401:7)
    x beforeEach failed on it should dispatch start and end actions: stub(obj,'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it should dispatch start and end actions, module: ext.popups/actions#abandon, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Have you spoken with #Design about changing this value?, expected: true,got: false, test: it should dispatch start and end actions, module: ext.popups/actions#abandon, source: at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:456:9)
    x Died on test #4     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/actions.test.js:433:7)
    x beforeEach failed on it shouldn't dispatch under certain conditions: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got: null, test: it shouldn't dispatch under certain conditions, module: ext.popups/actions#abandon, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x beforeEach failed on it should show the preview with the behavior: stub(obj, 'meth', fn) has been removed, see documentation, expected: undefined, got:null, test: it should show the preview with the behavior, module: ext.popups/changeListeners/render, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/changeListeners/render.test.js:14:7)
    x beforeEach failed on it should render the preview: stub(obj, 'meth', fn)has been removed, see documentation, expected: undefined, got: null, test: it should render the preview, module: ext.popups/changeListeners/render, source: TypeError: stub(obj, 'meth', fn) has been removed, see documentation
    x Died on test #2     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/changeListeners/render.test.js:40:7)
    x Died on test #1     at Object.<anonymous> (/Users/jhernandez/dev/wikimedia/vagrant/mediawiki/extensions/Popups/tests/node-qunit/wait.test.js:5:7)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ test:node: `node tests/node-qunit/run.js 'tests/node-qunit/**/*.test.js' | tap-dot`
npm ERR! Exit status 1

ac

  • tests are adapted to sinon 4.x
  • popups uses mw-node-qunit 3.x

Details

Related Changes in Gerrit:

Event Timeline

Jdlrobson triaged this task as Medium priority.Nov 13 2017, 9:56 PM
Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog-Archived board.
Jdlrobson subscribed.

Can we estimate the work involved here or do we need to do a timeboxed investigation to uncover the difficulty level here?

I think most of the failures are due to the removal of the deprecated signature of stub stub(obj, 'meth', fn).

I'd guess the migration work is going to be small, a couple of points.

For seeing the sinon warnings right now, edit the npm run test:node script and remove the | tap-dot part to see the raw TAP output, you should be able to see the deprecation warnings from sinon.

Jdlrobson set the point value for this task to 3.Jan 31 2018, 5:25 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog-Archived board.

We believe this should be a mechanical change and easy to verify, however there may be hidden complexity/risk in this task. It's likely to be a 1, but we want to accommodate for that risk, especially given many of the team have not workedwith this codebase.

@ovasileva: The FTEs have identified this task as being ready to work on in the next iteration.

Change 408995 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/Popups@master] Updating mw-node-qunit to v3

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

Change 408995 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Updating mw-node-qunit to v3

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

Looking good in principle, I'll try it out tomorrow and see if there is anything weird and sign this off

Checked locally, couldn't see any warnings anywhere on the test run (node tests/node-qunit/run.js 'tests/node-qunit/**/*.test.js') and the latest mw-node-qunit 👍