#297 Bundling exception for nodejs-expect-js
Closed: Fixed None Opened 6 years ago by jamielinux.

This is very similar to two previously accepted exceptions, in particular the discussion about the _deepEqual routine:

This module (like the modules in the other tickets) is often used in test suites. It once again takes code fragments from Node.js itself:

  • ~150 lines taken from Node.js's util.js
  • ~80 lines taken from Node.js's assert.js

Both sets of borrowed code are modified and do not work in exactly the same way as the original functions. Patching to use the original code is not really feasible as some of the functions are not made globally available, and modules that use expect-js for testing may see tests passing when they should actually be failing (or vice versa).

I would therefore like to request a bundling exception for nodejs-expect-js.


The language in the initial comment inclines me to vote for this but there's not enough information to be sure. Could we get the following additional information?

  • Link to the code
  • Link to the review request
  • Explanation of how the bundled code behaves differently
  • Does this code only get run in %check or is it also shipped to end users?
  • Is it run in other package's %checks as well?

Oops, sorry for the rather terse description. Here's the information that should have been there in the first place:
[[BR]]

Review request:
https://bugzilla.redhat.com/show_bug.cgi?id=911180

(NB: I've just renamed the package to nodejs-expect-dot-js!)
[[BR]]

Link to code:
https://github.com/LearnBoost/expect.js
[[BR]]

While the differences for assert.js are more subtle, the differences for util.js are quite significant and definitely not a lazy "copy-paste". If you compare the code, it looks like upstream have adapted the general structure of the original to form their own routine. Basically every function has been renamed and altered either subtly or significantly, and in some places additional functions have been added. There are too many differences for me to accurately summarize.[[BR]]

Lines adapted from util.js:
https://github.com/LearnBoost/expect.js/blob/0.2.0/expect.js#L553-770
Original:
https://github.com/joyent/node/blob/v0.11.2/lib/util.js#L204-455
[[BR]]

A few stages of the deepEqual routine have been modified to make different comparisons as well as returning different values. In particular, steps 7.3, 7,4 and 7.5 (as indicated in the code comments). Like the previous FPC exceptions, this code is different from the original and is also different from nodejs-deep-equal (a module designed to make the deepEqual routine publicly available), and is subject to change from upstream to suit their own needs.[[BR]]

Lines adapted from assert.js:
https://github.com/LearnBoost/expect.js/blob/0.2.0/expect.js#L843-934
Original:
https://github.com/joyent/node/blob/v0.11.2/lib/assert.js#L142-236
[[BR]]

Does this code only get run in %check or is it also shipped to end users?

This code is a core part of the module. The module essentially only consists of one file (expect.js), and the code mentioned above is embedded in this file (so yes, it does get shipped to end users). The code is likely run as part of it's own test suite within %check, though I haven't explicitly reviewed the code coverage in it's test suite.
[[BR]]

Is it run in other package's %checks as well?

The primary use of expect.js is as part of the test suite in other modules, so is likely to be used in the %check section of other packages.

info Bundling exception for nodejs-expect-js passes (+1:5, 0:0, -1:0)

It looks like for these code fragments the precedent has been not to record a Virtual bundled() Provide for them. So I'm closing this as fixed.

Login to comment on this ticket.

Metadata