There are some problems with nyc:
* The coverage numbers aren't useful in our case because most of the
code is executed outside the test process (the test code is mostly
API client logic).
* nyc messes with line numbers, which makes it much harder to debug
problems.
* We're seeing frequent SIGABRT crashes while nyc is printing the
results table. I'm not sure if nyc is the cause of the crashes, or
if it's making a race condition worse, or if the crashes have
nothing to do with nyc, but we don't lose much by removing it so
we might as well see if the crash frequency improves.
This makes it much easier to see why a test is failing. Before, a
`helper.waitFor()` failure would simply cause the test to time out.
Now an exception is displayed.
Before this change, the `author` attribute was silently discarded
during `.map()` iteration and the name of the attribute to remove was
included twice with two different values.
Before this commit, the callback passed to `.map()` during attribute
removal was a normal function, not an arrow function. This meant that
the value of `this` in the function body depended on how the callback
was invoked. In this case, the callback was invoked without any
explicit context (it was not called as a method, nor was it called via
`.call()`, `.apply()`, or `.bind()`). Without any explicit context,
the value of `this` depends on strict mode. Currently the function is
in sloppy mode, so `this` refers to the "global this" object (a.k.a.,
`window`). It doesn't make sense for the callback to reference
`window.author`, so I'm assuming the previous behavior was a bug.
Now the function is an arrow function, so the value of `this` comes
from the enclosing lexical context, which in this case is the
AttributeManager object. I believe that was the original intention.
This makes it possible for plugin backend tests to do
`require('ep_etherpad-lite/tests/backend/common')` to access the API
key (among other things).
Eventually we probably should reverse these (move `tests/` to
`src/tests/` and make `tests/` a symlink to `src/tests/`) and move
`bin/` to `src/bin/` so that we can avoid the top-level `package.json`
mess.
The `name` property is only available on cheerio's Element-like
objects; DOM Element objects do not have a `name` property. Switch to
`dom.tagName()` to fix the logic for browsers.
The `parent` property is only available on cheerio's Node-like
objects; DOM Node objects do not have a `parent` property. Switch to
the `parentNode` property so that the code works in browsers as well
as cheerio.
Before, the hook always ignored the return values provided by the hook
functions. Now the hook functions can change the text by either
returning a string or setting `context.text` to the desired value.
Also drop the `styl` and `cls` context properties. They were never
documented and they were always null.
In the DOM, `.children` only includes children that are Element
objects. In cheerio 0.22.0, `.children` includes all child Nodes, not
just Elements. Use `dom.numChildNodes()` and `dom.childNode()` so that
browsers behave the same as cheerio.
`for..in` iterates over inherited properties, which is almost never
desired. In most cases there aren't any inherited enumerable
properties so it's not that big of a deal, but in the case of
HTMLCollection it's very bad because it iterates over every entry
twice (once by numerical index and once by name) plus it includes the
`length` property in the iteration.
The `attribs` property is only available on cheerio's Element-like
objects; DOM Element objects do not have an `attribs` property. Switch
to `dom.nodeAttr()` to fix the logic for browsers.
`describe()` is meant to be used by independent tests, but the tests
in this file are not independent. Add a higher-level `describe()` call
and delete all of the `describe()` calls that wrap a single test.
