Draft: Write tests for license parsing in the WebLabels plugin.

I only wrote tests for a small part of the plugin to have your opinion on this. If it's fine, I'll write more.

Resolves #3122

Test Plan

npm test

---

Migrated from D1215 (view on Phabricator)

Build is green See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/185/ for more details.

Thanks for bootstrapping tests on this!

It was also in my mind to add some but I wanted to have a minimal working example on our frontend codebase before going further on the subject.

I intend to contact LibreJS authors next week to discuss about the approach and how the current specification could be improved to handle the contemporary way of working with JavaScript nowadays (aka bundling a lot of source code released under a free license).

As you noticed, it is not clear about how to handle multiple licenses and their combinations, this is pretty rare but such cases exist and they should be handled.

I hope that this work may be released for everyone on the npm registry and moved into a dedicated repository. Proceeding this way will allow to add proper tests on the plugin without polluting the swh-web repository.

Since the addition of cypress tests, we must ensure jest will not try to execute them. Modifying the command to jest swh/web/assets/* seems sufficient to do so.

mentioned in merge request !506 (closed)

mentioned in issue #3122

Sorry, I totally forgot that diff. I just rebased it to master and apart a small modification to the jest invocation, tests sill pass so looks good to me.

From my point of view, it can be landed as is and further tests could be added later if we need to update or improve the webpack plugin.

I contacted LibreJS authors a couple of months ago but unfortunately I did not get any answers on the licenses combination corner cases we faced. I have subscribed to the LibreJS mailing lists so I will check changelog on each release to see if there is some specification updates on these.

Just a nitpick before landing this, I enforce Javascript code style through the use of the eslint tool. I just added a yarn target (a09aa5fb) to run the tool and automatically fix what is easily fixable (indentation level, literal string style). In order for eslint to support jest, you will also need to install the eslint-plugin-jest package with yarn and update the "plugins" and "end" entries in the .eslinrc file of swh-web as explained here. So once the .eslintrc file is modified, simply executing the following commands will be enough to fix the eslint warnings:

$ yarn add eslint-plugin-jest --dev
$ yarn eslint

Merge request was accepted

approved this merge request

I used jest because it was the first search result when I looked for test frameworks.

As we use mocha for the frontend tests, maybe it makes more sense to make these tests use mocha instead of jest?

As we use mocha for the frontend tests, maybe it makes more sense to make these tests use mocha instead of jest?

This will be better for consistency with the other written frontend tests indeed and reduce the number of dependencies to fetch with yarn. Nevertheless, you will need to pick another assertion library (expect.js seems the simplest choice here to port the existing code) and mocking library (sinon.js seems the most widely used with mocha).

marked this merge request as draft

Jenkins job DWAPPS/gitlab-builds #157 failed .
See Console Output and Coverage Report for more details.

changed the description

closed

Jenkins job DWAPPS/gitlab-builds #309 failed .
See Console Output and Coverage Report for more details.