Page MenuHomePhabricator

MobileFrontend's headless qunit tests need Sinon support
Closed, ResolvedPublic3 Estimated Story Points

Description

While porting View.js and PageGateway.js to webpack (T203100+T203817), we hit several problems porting the tests.

The tests need sinon to work and adding sinon seems like a useful thing for our tests.

Acceptance criteria

  • The sinon interface is available, with the same version as MediaWiki core (and pegged to that version)
  • sinon is available consistently with how it made available in MediaWiki core (ie. if the tests run in Special:JavaScript/qunit/plain they run the same and pass the same as in headless mode
  • The code for sinon loaded inside our headless QUnit tests is not loaded in Special:JavaScript/qunit which already has its own version of sinon
  • Headless tests that reference sinon fail if sinon is not imported in the test itself

Per @Krinkle

The this.sandbox interface (also known as "sinon-qunit") is a superset that I think we do not need and it would simplify the work here. To do this, each test will be responsible to use beforeEach and afterEach to create and restore the sinon instance (one line of code). Doing that explicitly I think would improve readability and portability of the tests, as well as to feel more familiar to developers, including those not accustomed to MediaWiki-QUnit's particular way (you can blame me for that!).

Event Timeline

Jdlrobson moved this task from Needs Prioritization to Upcoming on the Web-Team-Backlog board.

I think it'd be better to relax the second AC to just "The sinon interface is available, with the same version as MediaWiki core" .

The this.sandbox interface (also known as "sinon-qunit") is a superset that I think we do not need and it would simplify the work here. To do this, each test will be responsible to use beforeEach and afterEach to create and restore the sinon instance (one line of code). Doing that explicitly I think would improve readability and portability of the tests, as well as to feel more familiar to developers, including those not accustomed to MediaWiki-QUnit's particular way (you can blame me for that!).

Jdlrobson set the point value for this task to 3.Sep 20 2018, 5:58 PM

@Jdrewniak @Niedzielski @nray and myself discussed and estimated this today.

Change 462811 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: add isomorphic DOM test utility

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

Change 462812 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Demo: Sinon.JS usage in headless Node.js tests

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

^ is this blocked on others or us? can you clarify?

@Jdlrobson, I thought we wanted to solicit feedback from @Jhernandez. I sent an email, presented our thoughts, and invited a conversation over email or Hangouts.

Change 462812 abandoned by Niedzielski:
Demo: Sinon.JS usage in headless Node.js tests

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

Change 462811 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: make headless test setups explicit

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

nray updated the task description. (Show Details)