SuperTinyIcons icon indicating copy to clipboard operation
SuperTinyIcons copied to clipboard

Update tests

Open kuubeu opened this issue 2 years ago • 4 comments

New tests:

  1. using Bun in place of Node.js with chai and mocha, reducing dependencies
  2. replaced validation using the W3C validator with a local runner, v.Nu, used by W3C; decreasing validation time from ~250 ms to ~19 ms per file (tested using GitHub Actions)

Notes:

  1. looks like Bun has a bit longer setup time than Node (~2–3 s instead of ~1 s), but hopefully it gets included by default on gh-actions runners in the near future, speeding things up
  2. the Java-based validator is a bit over 31 MB and currently gets re-downloaded every time, but bun install still only takes ~600 ms, compared to ~2 s of npm install with old tests

kuubeu avatar Sep 14 '23 17:09 kuubeu

I've verified that this works, and quite speedily, but I do have a couple outstanding concerns:

  • bun test isn't available on Windows without WSL yet. I'm not thrilled with the idea that we'd require WSL for people to debug their SVG errors. This is perhaps an uncommon and minor enough hurdle that it's fine, but I'm on the fence about whether this is worth the bump in test speed. (Appears to be actively worked on in oven-sh/bun#4410)
  • vnu-jar requires Java 8+ to be installed. It looks like everything will pass if you don't have Java on your path (a plausible situation) or if you have a really old version of Java (somewhat implausible)? Or maybe if there's I/O errors? I'd like to see the parsing of vnu output to be a little more robust, but I haven't really investigated the edge cases.

Eiim avatar Sep 14 '23 20:09 Eiim

Some thoughts on this:

  • As for the first point, is there really any need to go to Bun? We don't get the dependency slimming if we stay with Node, which I'll agree would be nice (chai+mocha was always overkill for this project), but switching to v.Nu does at least drop fetch anyways. It seems like the speedup here is probably mostly from running v.Nu locally rather than over the internet. To the extent that Bun is faster, I imagine it's something on the order of a second for this? Probably not worth the issues it brings with native Windows, at least for now, but some data on this would be nice if it's a major motivator.
  • Can we get v.Nu to output affirmative results ("xyz.svg is valid!") and check that we have those instead? Then if any are missing we could potentially dump the full v.Nu output, if that's not too absurdly verbose. I think that should cover all the likely edge cases and allow for easier debugging.

Eiim avatar Sep 15 '23 03:09 Eiim

When I implemented this I was only thinking of running the tests via Actions, and totally forgot that someone would want to run them locally (whoops!). I chose Bun as an all-in-one tool for testing and as a learning exercise

Addressing local testing: downloading v.Nu just to check one or a couple of icons might be a bit overkill, so I'm thinking of having local tests, fetching W3C like the original test.js and compatible with Node.js, but only on changed files, and running the new tests only on GitHub Actions. What do you think of this approach?

Also I'll look into v.Nu options later today!

kuubeu avatar Sep 16 '23 15:09 kuubeu

I don't think it's a big deal to download v.Nu (at ~32MB, it is larger than I expected, but not unreasonably so), and I'd rather use the same thing to test locally and with Actions, since my main motivation for local running is to debug problems spotted by Actions. In particular if validator.w3.org updates sooner/later than the npm repo, it could potentially cause some headaches. I do really like the idea of running v.Nu locally to make tests a lot faster, including through Actions.

Eiim avatar Sep 16 '23 15:09 Eiim