I'm not sure how these tests ever worked. I guess some version of
Node.js and npm come pre-installed on the ubuntu-latest images?
I would have prefered to use Node.js v10 because that is our current
minimum supported version, but we have a surprising number of tests
that don't work on Node.js v10 (mostly due to `assert.match()`, which
was added in Node.js v12).
Before, an unhandled rejection or uncaught exception during startup
would cause `exports.exit()` to wait forever for startup completion.
Similarly, an error during shutdown would cause `exports.exit()` to
wait forever for shutdown to complete. Now any error during startup or
shutdown triggers an immediate exit.
Logging verbosity of the openapi handlers was turned down so GitHub
should be happier with INFO now. This makes it easier to troubleshoot
problems.
This reverts commit b98aaf4904.
The settings commitRateLimiting.duration and commitRateLimiting.points
were not available in the settings.json.docker file, and therefore it
was not possible to override their values via environment variables.
Now, they can be overridden by setting the following env vars:
* commitRateLimiting.duration: COMMIT_RATE_LIMIT_DURATION
* commitRateLimiting.points: COMMIT_RATE_LIMIT_POINTS
`waitForPromise()` should always be used with `await` (either directly
or with a later `await` on the returned Promise). In this case,
the condition should be immediately true so `waitForPromise()` is not
the right tool here.
* Delete the unused `optDoNow` parameter from `pendingInit()`.
* Move the `setAuthorInfo()` 1st parameter check out of the wrapper
and in to the `setAuthorInfo()` function itself.
All of the tests in this file are commented out so this file does
nothing. We can uncomment the code and clean it up, but the approach
taken in these tests will never work: For security reasons, browsers
do not allow synthetic key events to perform the default
behavior (such as moving the carent when an arrow key is pressed).
There are two ways to test responses to navigation keys:
* Use WebDriver to create "genuine" keyboard events.
* Suppress the default behavior and implement caret movement
ourselves. This is tremendously complicated, especially arrow
up/down.
Advantages of a top-level `package.json`:
* It prevents npm from printing benign warnings about missing
`package.json` whenever a plugin is installed.
* Semantically, it is "the right thing to do" if plugins are to be
installed in the top-level directory. This avoids violating
assumptions various tools make about `package.json`, which makes
it more likely that we can easily switch to a new version of npm
or to an npm alternative.
Disadvantages of a top-level `package.json`:
* Including a dependency of `ep_etherpad-lite@file:src` in the
top-level `package.json`, which is required to keep npm from
deleting the `node_modules/ep_etherpad-lite` symlink each time a
package is installed, drastically slows down plugin installation
because npm recursively walks the ep_etherpad-lite dependencies.
* npm has a horrible dependency hoisting behavior: It moves
dependencies from `src/node_modules/` to the top-level
`node_modules/` directory when possible. This causes numerous
mysterious problems, such as silent failures in `npm outdated` and
`npm update`, and it breaks plugins that do
`require('ep_etherpad-lite/node_modules/foo')`.
Right now, with npm v6.x, eliminating the disadvantages is far more
valuable than keeping the advantages. (This might change with npm
v7.x.)
For a long time there was no top-level `package.json` and it worked
fairly well, although users were often confused by npm's benign
warnings. The top-level `package.json` was added because we needed a
place to put ESLint config for the stuff in `bin/` and `tests/`, and
we wanted the advantages listed above. Unfortunately we were unaware
of the disadvantages at the time. The `bin/` and `tests/` directories
were moved under `src/` so we no longer need the top-level
`package.json` for ESLint.
An alternative to a top-level `package.json`: Create
`plugins/package.json` and install all plugins in `plugins/`. If
`plugins/package.json` has a dependency of
`ep_etherpad-lite@file:../src` then plugin installation will still be
slow (npm will still recursively walk the dependencies in
`src/package.json`) but it should avoid npm's nasty dependency
hoisting behavior. To avoid slow plugin installation we could create a
lightweight `etherpad-pluginlib` package that Etherpad plugins would
use to indirectly access Etherpad's internals. As an added bonus, this
intermediate package could become an adaptor that provides a stable
interface to plugins even when Etherpad core rapidly evolves.
Also add symlinks from the old `bin/` and `tests/` locations to avoid
breaking scripts and other tools.
Motivations:
* Scripts and tests no longer have to do dubious things like:
require('ep_etherpad-lite/node_modules/foo')
to access packages installed as dependencies in
`src/package.json`.
* Plugins can access the backend test helper library in a non-hacky
way:
require('ep_etherpad-lite/tests/backend/common')
* We can delete the top-level `package.json` without breaking our
ability to lint the files in `bin/` and `tests/`.
Deleting the top-level `package.json` has downsides: It will cause
`npm` to print warnings whenever plugins are installed, npm will
no longer be able to enforce a plugin's peer dependency on
ep_etherpad-lite, and npm will keep deleting the
`node_modules/ep_etherpad-lite` symlink that points to `../src`.
But there are significant upsides to deleting the top-level
`package.json`: It will drastically speed up plugin installation
because `npm` doesn't have to recursively walk the dependencies in
`src/package.json`. Also, deleting the top-level `package.json`
avoids npm's horrible dependency hoisting behavior (where it moves
stuff from `src/node_modules/` to the top-level `node_modules/`
directory). Dependency hoisting causes numerous mysterious
problems such as silent failures in `npm outdated` and `npm
update`. Dependency hoisting also breaks plugins that do:
require('ep_etherpad-lite/node_modules/foo')
For some reason strings are sometimes passed to `findUnmet()`, which
is obviously unexpected given the way the code is written. Rather than
figure out why strings are passed and how to safely avoid passing
strings, just return early. The net effect is the same, but returning
early avoids setting a property on a string, which is prohibited in
strict mode.
