[Local Dev][Bug] - @oclif/core tests are not hermetic

[spanishpear]

Hi folks!

Context

I was writing #1363 and when running the local devloop outlined in CONTRIBUTING.md - the tests as per

yarn test

failed, as the oclif --version command was returning a different architechture

-  @oclif/core/4.2.10 wsl-x64 node-v20.19.1
+  @oclif/core/4.2.10 linux-x64 node-v20.19.1

Similarly the darwin tests fail, because, well - I'm not on darwin ;)

-/my/home/.cache/@oclif/core
+/my/home/Library/Caches/@oclif/core

Report

this resulted in 11 failing tests locally:

• main-esm > runs --version
• main-esm > runs --help
• main > should run version
• main > should run help
• Config > Darwin > should have darwin specific paths
• Config > win32 > should have win32 specific paths
• formatRoot > renders the root help
• formatRoot > description > splits on > for the description in the top level and description sections
• formatRoot > description > shows description from a template
• formatRoot > description > prefers the oclif description over the package.json description
• formatRoot > description > uses package.json description when the oclif description is not set

when running on WSL!

[git2gus]

This issue has been linked to a new work item: W-18398048

[mdonnalley]

@spanishpear Are you still seeing this? I don't have a windows machine to test on but the tests are passing for me on both mac and linux

Also worth mentioning that our unit tests run in CI against windows, mac, and linux - and they all pass. Although it could be that you're using WSL now that I think about it

If it's a WSL specific issue, I won't be able to find the time to fix this. Especially since we don't officially support WSL. However, I'd be happy to review and merge a PR if you are interested in submitting one

[spanishpear]

@mdonnalley Hey! Yeah, I just rebased and get the same failures

  10) formatRoot
       description
         prefers the oclif description over the package.json description:

      AssertionError: expected 'THIS IS THE OCLIF DESCRIPTION IN PJSO…' to equal 'THIS IS THE OCLIF DESCRIPTION IN PJSO…'
      + expected - actual

       THIS IS THE OCLIF DESCRIPTION IN PJSON

       VERSION
      -  @oclif/core/4.2.10 wsl-x64 node-v23.5.0
      +  @oclif/core/4.2.10 linux-x64 node-v23.5.0

       USAGE
         $ oclif [COMMAND]

      at Context.<anonymous> (test/help/format-root.test.ts:92:25)
      at processImmediate (node:internal/timers:511:21)

So it might indeed be something WSL specific ⁉
Will try and poke at it a bit more when I get some free time ❤

[spanishpear]

Okay, looks like the tests are actually failing as expected, because the tests are wrong 😅

---

There is code that explicitly sets this.platform = 'wsl', if it's running in WSL. This explains why the test failure doesn't reproduce on anything but WSL.

core/src/config/config.ts

Line 332 in 42f37b6

this.platform = WSL ? 'wsl' : getPlatform()

However the "expected" output for the test is setup to use process.platform instead, which correctly uses linux.

core/test/command/main.test.ts

Line 7 in 42f37b6

const version = `@oclif/core/${pjson.version} ${process.platform}-${process.arch} node-${process.version}`

As such we end with the following failure.

-@oclif/core/4.3.2 wsl-x64 node-v22.16.0
+@oclif/core/4.3.2 linux-x64 node-v22.16.0

So, the test fails... but because the expected output is incorrect, and not testing reality!

---

To make matters even more interesting, there are other failing tests that stub getPlatform

core/test/config/config.test.ts

Line 251 in 42f37b6

sinon.stub(os, 'getPlatform').returns('darwin')

but because the implementation is effectively

core/src/config/config.ts

Line 332 in 42f37b6

this.platform = WSL ? 'wsl' : getPlatform()

stubbing getPlatform is meaningless, as it doesn't even get evaluated 🤦‍♀

---

Tracing back, it seems it was added in oclif/config 1.16
as an intentionally distinguishable option from linux - oclif/config#106

There are only two usages of wsl as a identifier - so it's for third party consumers.
https://github.com/search?q=repo%3Aoclif%2Fcore%20%27wsl%27&type=code

---

Potential avenues forward...

• move wsl check into getPlatform, and mock accordingly (preferred)
• update the tests only to change their expected output to correctly test for wsl, keep implementation as is
• remove WSL support entirely (will regress WSL for config.platform config#106 🤷‍♀

I will probably submit a PR for the first option if you're happy with that direction @mdonnalley ?

[mdonnalley]

@spanishpear thanks for writeup! The first option sounds good to me. I'm also happy with the second option if the first doesn't work out