Modularize test runners

[savetheclocktower]

Have you checked for existing feature requests?

• [x] Completed

Summary

In #990 I wondered aloud if the eventual fix shouldn't be that Pulsar itself uses a modular test runner, just like any Pulsar package would need to do.

We were able to introduce Jasmine 2 alongside Jasmine 1 because the latter is vendored rather than being required via NPM (no doubt because it's patched to within an inch of its life), but if we wanted to introduce a third version of Jasmine — let's say version 5, the latest version — we'd be stuck. Jasmine 2 and Jasmine 5 can't coexist in node_modules.

However, if both versions of jasmine were transitive dependencies, it would work:

• A hypothetical @pulsar-edit/jasmine2-test-runner would have jasmine@2 as a dependency.
• A hypothetical @pulsar-edit/jasmine5-test-runner would have jasmine@5 as a dependency.
• Pulsar itself could depend on each of these two test-runner packages without any conflicts.

That's the technical reason to do it; the practical reason is that all the testing code is an utter mess. It's not the fault of #990; the code was a mess already.

The goal

I've started working on a jasmine5-test-runner package as described above. It's based heavily on @UziTech’s atom-jasmine3-test-runner, but is very opinionated about leaving some things out in the spirit of YAGNI:

• Clock mocking — I'm on record as hating it; it's a leaky abstraction; and Jasmine 5 has its own clock mocking that packages can opt into if they want to boil the ocean
• “tags” — Nothing philosophically against jasmine-tagged, but it doesn't seem like that feature was used very often
• profiling — Seems redundant when we have console.time and other built-in tooling

I'm also on the fence about

• JSON — I ported jasmine-json, but it feels borderline
• focused specs — Jasmine 5 has fit and fdescribe built in, and jasmine-focused changes the meaning of fit itself, but for now this is in

It seems to work great so far. But the catch is that, because it's based on Jasmine 5, it technically doesn't support any Node version below 18. That makes it a post-PulsarNext task — but that's a good carrot for us to get PulsarNext shipped.

What about mocking?

One of my unanswered questions from #990 was what to do about Pulsar setup code that assumes the ability to require things from the src directory. That's a privilege that the current test code has precisely because it lives alongside the source code — but it wouldn't have that privilege if it ran in a separate package!

This isn't a problem for the core Pulsar test suite. Even when it starts using a runner that it requires from an NPM module, it can add whatever custom mocks it likes in a beforeAll or beforeEach hook, and has access to all the stuff it wants to mock.

It does mean that Pulsar is unable to mock all that stuff for package specs, however — including the specs for builtin packages. Because any package could be using an arbitrary test runner other than Jasmine, we can't even ask Pulsar to handle the mocking itself, since it doesn't know the API or the conventions of the runner.

We'd talked about extending the test runner API to allow a package's test code to access these privileged modules, and that's still an option… but, honestly, I think we can fake most of what we need.

For instance: to convert some async editor tasks to synchronous tasks, the core test suite needs access to TextEditor, TextEditorElement, and both the language mode classes. TextEditor is part of the public API of Pulsar and can be accessed at require('atom').TextEditor, and once you have an instance of a TextEditor, it's not hard to grab references to TextEditorElement and TextMateLanguageMode, even though they're not exposed. (WASMTreeSitterLanguageMode is slightly harder, but still doable.)

I'm confident we can start out with clever workarounds like this. I'm also pretty confident that most of the mocking we do is as outdated as the clock mocking, and hard to justify in an async/await world. Any package that used atom-jasmine2-test-runner or atom-jasmine3-test-runner managed to get by just fine without those mocks in the first place, so this isn't something worth fretting over.

What benefits does this feature provide?

• Core suite would get modernized to Jasmine 5; this is labor-intensive, but it's an opportunity to revisit a lot of specs that haven't been properly rewritten (only decaffeinated!) perhaps ever
• We'd also spin the existing Jasmine 1 code into its own @pulsar-edit/jasmine1-test-runner package; it could exist in its current messy state, but in isolation
• Packages could opt into any of these runners without having to install the associated package (though they could if they wanted to!), or else would default to the Jasmine 1 runner if that field were omitted

Any alternatives?

In technical terms, no. The best way to introduce new arbitrary test runners is to remove the need for multiple versions of the same package to coexist as dependencies.

Other examples:

🤷

[UziTech]

For instance: to convert some async editor tasks to synchronous tasks, the core test suite needs access to TextEditor, TextEditorElement, and both the language mode classes. TextEditor is part of the public API of Pulsar and can be accessed at require('atom').TextEditor, and once you have an instance of a TextEditor, it's not hard to grab references to TextEditorElement and TextMateLanguageMode, even though they're not exposed.

If you can already get access to it, why not just make it public? I think it would be better to make everything public (since this is a hackable editor). If you really wanted it just for testing maybe put it behind a namespace like import { TextEditorElement } from "pulsar/testing";

[savetheclocktower]

If you can already get access to it, why not just make it public? I think it would be better to make everything public (since this is a hackable editor). If you really wanted it just for testing maybe put it behind a namespace like import { TextEditorElement } from "pulsar/testing";

Also an option! But I think it can wait, as I'm curious how much of it is truly needed.

After staring at it for a long time, I think there are only a handful of places where it really makes sense to mock APIs (like suppressing file save dialogs). If it's important to mock those things, then I think it makes sense to put them in more accessible places, and perhaps even to make the mock part of the API itself. (Imagine a TextEditor::setFileSaveTestMode(true) method call to turn that behavior on at the beginning of a test suite.)

It's not quite the same thing, but WASMTreeSitterGrammar has a backdoor into the Tree-sitter query system for this reason. There's a setQueryForTest method that gives us a quick way to, e.g., redefine an existing grammar's highlighting query. This is mainly useful for tests, as the name implies, but I've also suggested it a few times to users in Discord who want to do something funky with their syntax highlighting.

[savetheclocktower]

OK, I've added my work so far at @pulsar-edit/jasmine5-test-runner.

When I'm able I'll see if I can write @pulsar-edit/jasmine2-test-runner in the same style, then @pulsar-edit/jasmine1-test-runner. Ideally I'll be able to migrate the existing Jasmine test situation in the main codebase over to these latter two packages while maintaining the current behavior.