Cleanup/modernization of developer test/build tooling

[Mr0grog]

This is a follow-on from #190, where I did some basic work to move the dev tooling forward just a bit and resolve some [slight] potential supply-chain style security risks. There were a bunch of open questions about what is worth doing or should be done for dev tooling going forward, and it seems better to have an open issue for this rather than discussion in a completed PR.

So, continuing the discussion from #190 (comment) …

while [more updates] are very helpful, I don't think it's worth trying to incrementally update this further for modern node etc.

FWIW, I would like to try and update a little further here (that is, I am happy to do the work if you don’t mind reviewing), since it is still tough to get Node.js 10 set up on a Mac with an Apple/ARM CPU (i.e. my main computer 😉). I also think enabling tests on current versions of browsers and Node is good because as much as JS engine developers don’t want to break the web/break existing code, sometimes it happens.

Also, I just noticed that the test suite is currently passing in PhantomJS, but not in modern browsers (I had not tried viewing Jasmine tests manually via the integration-test task before). So that’s probably good evidence this needs some more updates. 😬

It's going to be a significant amount of work, and it would probably be better spent just replacing the test setup entirely, to rewrite it completely with modern tools. Or, more realistically, as loglevel is mostly in 'done' maintenance don't-change-too-much mode

Yeah, agree that loglevel is mostly in maintenance, and working too hard on this, or making changes that are too big are probably not worthwhile. What I think is worth focusing on here is making the DX tooling a little more resilient to to ravages of time:

• Update to the latest Grunt. Grunt is in a similar “pretty stable and done, but still well maintained for security/bugs” status as loglevel.
• Or drop Grunt if that turns out to be easier than maintaining a Grunt-based setup [because of outdated plugins]. I think I’ll discover which is easier pretty quickly when doing the work.
• We probably don’t want really fancy new tooling, because that tooling is more likely to change a lot need a lot of updates by the next time a feature or fix needs to reviewed here.
• Basically: minimal tooling with minimal tooling-caused churn.

Having a test suite working on modern Node & browsers also makes every change and bugfix PR submitted here more safe. That’s only more valuable for something in maintenance mode.

In any world where somebody has the time & interest to build out loglevel v2 and take this forward with more substantial changes, I think the first step would be creating a new build env for modern node & tools, starting from a mostly clean slate.

FWIW, I think having a working and maintained test suite to start from is really useful here, whether someone starts the library code from a clean slate or by making a lot of incremental changes. If they don’t want the public behavior (and thus the test suite) to mostly stay the same (of course there will be some behavior changes), they probably aren’t building a logger that should be called “loglevel v2,” IMO.

while the package aims to support v0.6.0 at runtime (entirely because updating that would be a breaking change) it's not the end of the world if the dev environment requires a later version.

👍 Given that, some quick tooling ideas on how to provide safety around v0.6.0 support:

• Switch from JSHint to ESLint + eslint-plugin-compat. You can set what JS version/syntax you support in ESLint, and the compat plugin can help check for unsupported API use based on minimum browser and Node.js versions.
• Add some really basic smoke tests (on the same level as the tests currently in global-integration.js or node-integration.js tests) that don’t rely on a test tool at all, and can just be run against the build products sitting in dist/. These could then reliably run in all supported versions of Node or browsers, while the more extensive full test suite only runs in more modern stuff.

I think either or both of the above would add some more safety here, although I definitely hear you that reliable support testing on such outdated runtimes is probably not critical.

A couple other questions I had about tooling:

• The QUnit tests seem very duplicative. Are they just there to test that loglevel doesn’t break when a consumer uses QUnit to test their code (loglevel cannot be used with qunit #46)? (If so, it might be worth dropping as long as we are testing noConflict() elsewhere, or at least adding some comments to the test describing that it’s just a smoke test for QUnit as an environment, and not a place any sort of new tests should generally be added.)

• Are you still using Travis anywhere behind the scenes? I know they dropped their free-for-open-source service a few years back, and don’t see any checks showing up on PRs. Should I drop the Travis config (and replace with GitHub Actions)?

• Are you still using Saucelabs? If not, we can drop all the plugins and Grunt tasks for it.

• Are tests still running in any environment where test/vendor/json2.js is needed? Otherwise we could drop it.

• Are you still using coveralls.io? Otherwise we can drop that stuff, too. It doesn't seem like anything is reporting back publicly on GitHub, and when I run the Grunt task it reports: >> Failed to submit 'coverage/lcov.info' to coveralls: Bad response: 422 {"message":"Couldn't find a repository matching this job.","error":true}, but that could just be because I don’t have credentials.

  • Relatedly, the jasmine:requirejs and jasmine:withCoverage test tasks appear to cover all the same tests, just with or without coverage tracking. It seems like we should only run one of them (which one depends on whether you are actually still tracking coverage).

[Mr0grog]

Also, I just noticed that the test suite is currently passing in PhantomJS, but not in modern browsers

Will try and submit a fix for this later today, although the real point here is: we clearly and running and checking that things are passing in up-to-date environments, and there are bugs in the library as a result.

[Mr0grog]

PR for the above bug and other test failures in #192.

[pimterry]

Ok, covering your questions first:

The QUnit tests seem very duplicative. Are they just there to test that loglevel doesn’t break when a consumer uses QUnit to test their code?

Yes, this was about a decade ago now (!) but I'm pretty sure that was the reasoning. Happy to drop these and cover noConflict elsewhere instead.

Are you still using Travis anywhere behind the scenes? I know they dropped their free-for-open-source service a few years back, and don’t see any checks showing up on PRs. Should I drop the Travis config (and replace with GitHub Actions)?

No - very happy to replace with GitHub Actions if you're up for that.

Are you still using Saucelabs? If not, we can drop all the plugins and Grunt tasks for it.

No - happy to drop that.

Are tests still running in any environment where test/vendor/json2.js is needed? Otherwise we could drop it.

No - looking back at the history I'm not totally sure why that was included originally, but I'm not aware of anywhere it's currently used.

Are you still using coveralls.io? Otherwise we can drop that stuff, too.

See https://coveralls.io/github/pimterry/loglevel

This was used early on and was very helpful, but it's not especially important now imo, and I don't know how to recreate credentials & configuration to work out how to re-enable this for any new builds.

Happy to drop it if that's easiest. If the existing coverage reporting is working, we may as well keep that (but without duplicating the testing, as you suggest) but only if it's easy - any substantial work to do so is not worthwhile imo.

---

In general, quick easy fixes are welcome here, and I'm happy to review and discuss those. I don't think seriously substantial work here within these constraints is especially valuable though.

Given the simplicity of the library, the big changes in the ecosystem since it was originally created (IE6 & Node 0.6.0 support are definitely no longer important!), it seems more sensible to solve "how do we ensure that loglevel works in v0.6.0" by just never making changes again. It works well enough now, and it's been largely unchanged for the last few years already. Easy peasy.

That said, it seems you're full of beans and ready to make improvements, which is great! Would you be interested in spending that time to kicking off a v2 here, instead of putting this work into v1? That would allow a lot more freedom to ignore some concerns here - e.g. we could immediately drop Node 0.6 support, remove UMD/requireJS, reevaluate details of API design, etc. Happy to discuss details, probably in a fresh issue after closing #119 which is now quite outdated itself.

I would still aim for wide compatibility & lightweight minimalism, but matching modern norms (i.e. just CommonJS & ESM formats, targeting LTS Node+Bun+Deno+modern-ish browsers, etc). v1 could then remain frozen unless any major issues appear (v unlikely at this stage).

I do agree with your point that re-using tests for v2 and preserving API compatibility where possible would be helpful. However, I think preserving the test code but migrating libraries & runners directly to the simple modern options (e.g. npm scripts + mocha + chai) should be very possible, and that would avoid many of the more fiddly challenges here.

[Mr0grog]

OK, I'll add GitHub Actions first (so everything else gets tested), then do a PR to drop some of the vestigial stuff.

[Coveralls] was used early on and was very helpful, but it's not especially important now imo, and I don't know how to recreate credentials & configuration to work out how to re-enable this for any new builds.

D'oh, I totally missed the coverage shield at the top of the README!

If you aren’t able to log in anymore, I’ll cut all that stuff out. But if you are, maybe worth keeping it? Or switching to this official Action step they have: https://github.com/marketplace/actions/coveralls-github-action Depends on whether you want to bother, since you’ll have to do the integration since it's your credentials/you control the workflow secrets in GitHub/etc.

---

Re: supporting older versions of Node.js and browsers, it sounds like adding CI tooling for that is probably more work than you want to support, so I won’t worry about it, and I also won’t worry about any dev dependency changes that increase the minimum version of Node.js required. (I will try and add something about this to CONTRIBUTING.md, though.)

---

Would you be interested in spending that time to kicking off a v2 here, instead of putting this work into v1? That would allow a lot more freedom to ignore some concerns here

I’ve had some time this week to work on this, which I’m happy to keep doing. That said, I don’t think I have the energy or time for the kind of full v2 rewrite you’re suggesting here, especially since I am not in charge here and there would inevitably be a lot of back and forth with you over the right vision/implementation. If I started down this path, I think it’s highly likely the work would peter out halfway like #119 did.

That said, I am curious what your goals for a v2 would be. Everything you’ve mentioned here sounds very internal facing and aimed at cleaning up the codebase/reducing maintenance burden. Are there external-facing things you want to change, too? Otherwise, maybe the right path here is that you simply declare the next release to be v2, and the only real difference is that you're changing compatibility/support expectations in order to make future bugfixes/maintenance easier. Then you can update internals at any pace you want. (And if you don’t think you want to do that rewriting work — at any pace — you don’t need to, but then maybe there’s not a strong reason for a v2 at all? Or you might consider finding someone who wants to take it over from you? Just putting forward some thoughts.)

---

I think preserving the test code but migrating libraries & runners directly to the simple modern options (e.g. npm scripts + mocha + chai) should be very possible, and that would avoid many of the more fiddly challenges here.

Having done this kind of change before, I suspect keeping on the current path with Jasmine and eliminating some of the vestigial tooling (that you’ve already confirmed can be dropped) will be easier (Chai in particular has a lot of subtleties that differ from Jasmine assertions in ways that can require some careful work). But changing the test tooling is probably worthwhile if you are actively planning to make bigger internal changes. I think I can probably work on this if you want, but whether it makes sense really depends on how you feel about the v2 things discussed above (are you going to work on that even if I or some other external contributor is not doing a lot of the work with you? Otherwise changing the test tooling has little payoff).

[Mr0grog]

Added the GH Actions workflow in #193.

[pimterry]

I’ve had some time this week to work on this, which I’m happy to keep doing. That said, I don’t think I have the energy or time for the kind of full v2 rewrite you’re suggesting here [...] That said, I am curious what your goals for a v2 would be. Everything you’ve mentioned here sounds very internal facing and aimed at cleaning up the codebase/reducing maintenance burden. Are there external-facing things you want to change, too?

Yeah, fair points & good questions throughout.

I do think there are external-facing things that could be clearly improved in the API, from the plugin model to all sorts of small details of other API behaviour (some of which we've discussed: rebuilding automatically when setting logs, redefining 'default' log levels, etc).

In each case, those were quick decisions that weren't great design choices at the time, but can't now be changed without a major bump.

Batching all those changes and internal updates into a single v2 update would be nice. I'd rather not ship a quick v2 containing just the internal changes without those improvements though - it's hard to make a good case for a release where the only change is reducing compatibility by updating all the dev tools. There's little point in updating those unless the larger work is going to follow on after (and given an internal-update-only v2, that would have to come as a v3 directly afterwards).

Right now, I don't really have the energy or time to invest in that myself. I'm super busy at the moment, and loglevel hasn't been a major focus for a good while. It's largely running in maintenance mode - very widely used, and I'm keeping an eye on it for major issues, but I've only really got active time for essential fixes & similar.

Given all this, I think for now we should punt anything that doesn't fit into the 'easy & fully compatible fixes', and all other v2 & major improvements can remain on hold indefinitely.

You're welcome to open PRs to improve things within those constraints, if you have the time, but let's not go further than that please - there's little value there I think for now, and I think there's a risk you waste your time here if we take this much further.

[Mr0grog]

I think for now we should punt anything that doesn't fit into the 'easy & fully compatible fixes'… You're welcome to open PRs to improve things within those constraints, if you have the time, but let's not go further than that please

Makes sense. Here’s what I’m thinking I’ll do to finish out this issue:

• Update dev tooling to work on current versions of Node.js (v18/v20). That way anybody contributing a small bugfix or other patch can test it without too much effort. (Get tests and builds working in current Node.js versions #195)
• If not too complex, update dev tooling to run in Headless Chrome or Firefox (so some of the tests that currently skipped because of Phantom issues can actually run). I think this will come for free with the above changes, so it shouldn’t be a big deal. If it does turn out to be big, I’ll note that in this issue and stop working on it. (Get tests and builds working in current Node.js versions #195)
• Add some notes about the Node.js version required for dev and difference between that and the versions of Node.js/JavaScript that the runtime code targets to CONTRIBUTING.md. (Final docs and audit cleanup #198)
• Finish out Add Logger.rebuild() Method #189 (not really relevant to this issue; mainly clarifying if it is in-bounds for the scope you want to keep here)

Does that seem reasonable and fit within the constraints you’re looking for?

[pimterry]

Does that seem reasonable and fit within the constraints you’re looking for?

Yep, that all sounds good to me 👍