Get tests and builds working in current Node.js versions

Mr0grog commented 10 months ago

This gets tests and builds working in current versions of Node.js, from v6 through v20 (current active release), and in MacOS on Apple Silicon. The main goal here is to make it possible to run tests and builds on most types of computers without lots of complex setup, so it’s not too hard for contributors to test and validate their work.

⚠️ There are some messy bits here, and you might want to see some changes here, or might feel any solutions here are too complex and this PR should not be merged at all.

The main changes here are:

Upgrade grunt-contrib-jasmine to v4.0.0 and grunt-template-jasmine-istanbul to v0.6.0 (via git, not NPM). Unfortunately, grunt-template-jasmine-istanbul now points to the git commit for v0.6.0 instead of NPM, because this version was never published on NPM. I believe that is because of some potential issues with tests in the final change (it looks like nobody ever verified whether actually are issues — the author supposes they might be resolved when another, parallel PR merges, and that PR did in fact get merged).

Some options on making this situation better:
1. Drop grunt-template-jasmine-istanbul entirely. This means we lose test coverage data, but we aren’t currently using that data for anything anyway. (I think, given the current status of things, this is the best option. I didn’t do it yet in this PR to keep things clear and minimal.)
2. Vendor this version of grunt-template-jasmine-istanbul so we aren’t relying on a git repo or commit that could disappear.
3. Downgrade to grunt-contrib-jasmine v1.2 + grunt-template-jasmine-istanbul v0.4.0. This is less ideal because this version of grunt-contrib-jasmine still uses PhantomJS, which has been dead for 6 years. Eventually it will probably start having problems in newer systems. This version also has a critical command injection vulnerabiltiy, and would reverse some of the work I went through to clean those out in #190. You can see an implementation of this in the first commit of this PR (8a4a74406beafad4523bc650fc1bbbf1e2f193e8).
Remove grunt-jasmine-node in favor of using jasmine directly. The new, inline Grunt task for calling Jasmine directly is extremely simple, and I think it poses low maintenance overhead. It also removes some security issues and allows us to use the same version of Jasmine in both Browser-based tests and Node.js-based tests.

An alternative here is to use grunt-jasmine-node v0.3.1, but it requires a ridiculous hack because it turns out to be totally broken with respect to loading configuration options. More info in this commit that tries using it: 94acb4ec900aa4ae3ea85485bacb5e88c2a886f4. All versions of grunt-jasmine-node that work in modern Node.js versions have this problem. I would definitely recommend against this.

This completes the first two items from https://github.com/pimterry/loglevel/issues/191#issuecomment-1894653099.

Mr0grog commented 10 months ago

Realized today that in 72182e5e5deb7a8540c7e02f168ee4cbc4669d62 when I updated the version of grunt-contrib-jasmine to v4, it also upgrade Jasmine to v5.1 for browser tests. I figured I should upgrade the Node.js tests to match, which was pretty simple, but then realized those require at least Node 18+, which seemed like a bit too big of a jump, so I reversed that. This is easy to put back in with a single git revert bb00d4193688255c39544cabf493e4bad461651e, though.

Along the way, I also noticed that cookie support checking in the tests is not actually working correctly, resulting in some tests getting skipped when they shouldn't be (this was fine in PhantomJS since it doesn’t properly support cookies, but different in headless Chrome or in live tests with grunt integration-test). I fixed that check in 160fc4cce49e9607148d23fc6e5c9b17099c0553.

pimterry commented 10 months ago

Some options on making this situation better:

Drop grunt-template-jasmine-istanbul entirely.

Yeah, I'm fine with that. It seems like the current approach adds complexity (requiring legacy peer deps options, using unofficial releases, depending on a now-unmaintained dead project) and as we discussed in #191 we're not actually using the coverage data now.

If it was easy to keep working that'd be fine, but this seems more hassle than its worth, so I'm happy to cut it entirely and streamline.

Mr0grog commented 10 months ago

Makes sense. That should be done, and this PR should be ready to go.

pimterry / loglevel

Get tests and builds working in current Node.js versions #195