Testing Improvements

[lishaduck]

• [ ] Don't make tests need internet/GHA API
  • [ ] Migrate to octokit?[^1] (we get types to replace tests)
• [ ] Fix flag parsing and enable memory leak tracking (https://github.com/jfmengels/node-elm-review/issues/156#issuecomment-2184082300)
• [x] Fix "error: conflicting output format options" from diff (#193)
• [x] #197
• [x] #224
• [ ] Don't break snapshots on new Elm package releases (ref: #185) 🌟
• [ ] Maybe migrate to Vitest, as it supports #171?
• [ ] Don't shell out, call the main method (speed) (requires a rework of options parsing, so long term)
• [ ] #313
• [ ] 100% Code Coverage (stretch goal; requires not shelling out)

[^1]: Alternatively, ungh would help with rate limiting and types once ofetch finishes adding in support for statically typing API endpoints (https://github.com/unjs/ungh/issues/4#issuecomment-1401762324).

[lishaduck]

I don't know if this is visible without a GitKraken account, but here's a patch for offline testing for now: https://gitkraken.dev/link/drafts/ba02a3a7-4beb-4ed9-aac8-447540488f70?type=patch

[lishaduck]

Yay! One of my issues was "yet another longstanding NPM bug™," so it can be removed from this list, at least.

[jfmengels]

I don't know if this is visible without a GitKraken account, but here's a patch for offline testing for now:

It is not, unfortunately.

[lishaduck]

I don't know if this is visible without a GitKraken account, but here's a patch for offline testing for now:

It is not, unfortunately.

I figured. Why do they have to overwrite copy-as-patch? Well, there's the actual patch:

diff --git a/test/run.sh b/test/run.sh
index 86bccb7..1c38065 100755
--- a/test/run.sh
+++ b/test/run.sh
@@ -163,9 +163,9 @@ else
   echo -e '\x1B[33m-- Testing runs\x1B[0m'
 fi

-PACKAGE_PATH=$(npm pack -s ../ | tail -n 1)
+PACKAGE_PATH=$(npm pack --offline -s ../ | tail -n 1)
 echo "Package path is $PACKAGE_PATH"
-npm install -g $PACKAGE_PATH
+npm install --offline -g  $PACKAGE_PATH

 # init

diff --git a/turbo.json b/turbo.json
index 92ac8f8..82d6d2c 100644
--- a/turbo.json
+++ b/turbo.json
@@ -18,8 +18,7 @@
         "tsc",
         "eslint-check",
         "prettier-check",
-        "jest",
-        "test-run"
+        "jest"
       ]
     },
     "eslint-check": {

And the message:

The turbo.json changes turn off github and npm api access. The run.sh changes try to turn off npm api access, but make some tests fail.

[lishaduck]

FYI, tests are all failing in CI right now. Not sure why.

[lishaduck]

FYI, tests are all failing in CI right now. Not sure why.

Finally got a chance to check this out, new elm-syntax broke tests again.

EDIT: Fixed on #197

[lishaduck]

Finally got a chance to check this out, new elm-syntax broke tests again.

And again, in 7.3.5 😢

[lishaduck]

I wrote a quick branch swapping to vitest last night, and it's fast. The IDE integration is superb, and test time was quartered.

Disclaimer: only supports node 18+, so not really an option right now. Branch: https://github.com/jfmengels/node-elm-review/compare/main...lishaduck:node-elm-review:vitest (it's actually a smaller diff, but I made tests concurrent that were safe to)