Page MenuHomePhabricator

[Spike][4hrs] Investigate replacing Istanbul with nyc
Closed, ResolvedPublic0 Story Points

Description

We used Istanbul to generate code coverage reports in Popups. It's barely maintained now and broken.

The proposal is to switch to https://github.com/istanbuljs/nyc but it's not clear how to migrate that and whether it will work with our setup.

your mission if you chose to accept it - take some time to investigate whether this is feasible and what the challenges are for doing so

Right now, Joaquin is the only one who understands this so we need to knowledge share so he's not a single point of failure.

Output:

  • Are there any challenges or changes needed with node-qunit
  • Migration plan OR Gerrit patch showing the necessary changes

Sign off steps

  • Resolve or unstall T193519 as appropriate

Event Timeline

Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from To Triage to Upcoming on the Readers-Web-Backlog board.
Jdlrobson set the point value for this task to 0.Jun 12 2018, 4:21 PM
Jdlrobson updated the task description. (Show Details)

This task may or may not require changes to @Jhernandez's mw-node-qunit. If we have any trouble, let's ask Joaquin for guidance.

As it stands now it that is only a wrapper around the official qunit so there may be issues with the upstream of any. It is worth checking any problems against raw qunit to see if it is our wrappers fault or not.

Niedzielski claimed this task.

Change 440904 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Fix: code coverage

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

Niedzielski removed Niedzielski as the assignee of this task.

To whomever reviews the above patch: if you're happy with it, consider resolving T193519. I just used the ASCII report since it seemed reasonable and would be easy to eye in CI.

Change 440904 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Fix: code coverage

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

Jdlrobson removed a project: Patch-For-Review.
Jdlrobson removed Jdlrobson as the assignee of this task.
Jdlrobson added subscribers: pmiazga, Jdrewniak.

@pmiazga or @Jdrewniak can one of you sign this off?

Yup. Just ran the test and the coverage report works :)
Signing off

Jdrewniak closed this task as Resolved.Jun 26 2018, 7:20 PM
Vvjjkkii renamed this task from [Spike][4hrs] Investigate replacing Istanbul with nyc to f8aaaaaaaa.Jul 1 2018, 1:04 AM
Vvjjkkii removed Jdrewniak as the assignee of this task.
Vvjjkkii reopened this task as Open.
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii removed the point value for this task.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot set the point value for this task to 0.
CommunityTechBot lowered the priority of this task from High to Normal.
CommunityTechBot closed this task as Resolved.
CommunityTechBot claimed this task.
CommunityTechBot reassigned this task from CommunityTechBot to Jdrewniak.
CommunityTechBot renamed this task from f8aaaaaaaa to [Spike][4hrs] Investigate replacing Istanbul with nyc.
CommunityTechBot added subscribers: gerritbot, Aklapper.
CommunityTechBot added a subscriber: CommunityTechBot.