[core] Replace Jasmine v1 Test Runner with new repo - #1525
confused-Techie wants to merge 1 commit into
Conversation
|
Adding myself as a reviewer so that I don't forget about this. Also gave it a 1.134.0 milestone so that it gets reviewed eventually (we've already got a lot of stuff in the hopper for 1.133.0!). |
|
I'll try to give this proper review tonight. But in the meantime: this is clearly the direction in which we want to go, so feel free to transfer My dream is to have this and Jasmine 2 (the status quo for Pulsar's own non-package specs and one or two core packages) and Jasmine 5 be available as modular runners like this. I am not certain whether a community package would be able to specify the runner by package name and have it recognize the runner's ambient availability — that needs research. But if not, I figured we could have some sort of magic syntax that means “use this specific runner that I know to be bundled with Pulsar” — like |
savetheclocktower
left a comment
There was a problem hiding this comment.
So this appears to work great, but there are a couple of snags:
- Let's definitely get the repo transferred over to
pulsar-editand publish it; then we can update the reference here. - The graphical runner is broken because of a
requireof a nonexistent path; see here.
One other issue that Claude caught and which I consider minor: We create a TextEditor in the spec window in order to capture references to TextMateLanguageMode and TextEditorElement, but then we never destroy the editor. This is pretty minor, since it's a leak of only one editor per run.
As discussed on Discord, this PR replaces all of our existing
jasmine1-test-runnerlogic with my working repositorypulsar-jasmine1-test-runner.Doing this brings quite a bit of code (that we likely will never touch due to compatibility) out of core and siloed into it's own repo.
Why do this?
Currently, our Jasmine 1 test runner, is honestly wildly complex. And if you wanted to track down how it all worked, boy were you in for a bad time.
Without getting too into the weeds of it, but to truly understand how we defined Jasmine, you'd have to:
./spec/helpers: Has several different modules that add new stuff or change stuff of Jasmine./vendor/jasmine.js: Seems like it'd be our main jasmine definition, but it's actually mostly redefined later../vendor/jquery-jasmine.js: More special stuff on top of Jasminejasmine-focused: Another repository that does more special stuffjasmine-json: Another repository that does more special stuffjasmine-reporters: Another repository that does more special stuffjasmine-tagged: Another repository that does more special stuffjasmine-node: Now it gets tricky, this isn'tmheverys repository like you might think, it's a branched version made bykevinsawickithat we technically import fromjasmine-focusedthat we define over itself a couple of times. Even more complicated we technically define it over top of our own vendored Jasmine, but the two copies are nearly identical, except about 18 additions bykevinsawickiand about 276 additions on our vendored copy.And keep in mind with how much redefining we do over top of things already defined in the global namespace the order of these changes/redefinitions matters a great deal.
Plus each new repository has many of the same modules of varying versions that are all out of sync with each other.
So to finally answer the why, bringing in a single module of
pulsar-jasmine1-test-runnermeans we can archive all of these separate repos, and have one set of modules to keep updated within this other repository, and means we can put all of the code that defines ourjasmine1runner in one place.Even though in that repo I kept the structure the same, meaning there's still a lot of bouncing back and forth, and redefinitions, I still feel the advantages outweigh the disadvantages. Especially when we consider that we will likely never change how this test runner functions, we would only move to update the test runner like we already have with our
jasmine2-test-runnerbeing what's used in the main Pulsar repository.Now I will specify, if we like what this PR does and the general shape of my working example of the
pulsar-jasmine1-test-runnerrepository, I'd recommend that prior to any kind of merge we fork my repo to Pulsar, and publish to NPM as@pulsar-edit/jasmine1-test-runnerthen make sure we use that version here. Just getting this up as a way to get this idea reviewed in some detail.And if we don't like the shape of my repository I'm already having to stop myself from trying to clean it up too much, since while I'd want to, I would be pretty nervous about breaking something in a subtle hard to catch way, but I'd still be up for cleaning it up if we really wanted too