CI should build Pulsar and run smoke tests

[savetheclocktower]

Have you checked for existing feature requests?

• Completed

Summary

This recent saga with libiconv on macOS illustrated that, up until now, we'd been building Pulsar with the version of superstring that the Atom folks most recently published to NPM — not the version from our fork against which we've applied a number of commits.

When we pointed Pulsar at the head of our fork's master branch, a number of issues were revealed, including very consistent crashes of Pulsar's renderer process during editor and package tests. When we assembled an alternative branch — one consisting of all our changes minus anything that affected the core superstring source code — and built Pulsar against that, all our problems went away.

So some of these commits caused breakages in Pulsar. We didn't know at the time, but we were bound to discover it eventually. This strongly suggests that it's not enough for us to run superstring’s own tests to prove that it works; we should also be running some smoke tests against Pulsar to prove that a proposed change will not regress Pulsar's behavior.

What benefits does this feature provide?

The immediate benefit is that it reveals much sooner in the process whether a change to superstring will cause a regression in Pulsar.

The medium-term benefit is much larger: at some point we'll need to have a version of superstring that can run in the most recent version of Electron. That requires a migration to N-API (see #10). At that point it's not just about regressions! It's about threading the needle of being able to support both Electron 12 and Electron ~30 (and climbing) with one version of superstring — or, if that's not possible, maintaining two different versions at once.

Either way, this work will necessitate a second set of Pulsar smoke tests in CI: one that tests against an experimental version of Pulsar built against a recent version of Electron.

Any alternatives?

I don't really see a way around this. The status quo is an alternative — one in which we don't find out how a given change affects Pulsar until later in the process — but that feels self-evidently worse.

Other examples:

No response

[DeeDeeG]

I can at least say @mauricioszabo appears to be fairly comfortable working with various dependencies (such as superstring) as git repo URL dependencies, off of specific "working (WIP) branches" of repositories as needed. This may be the "status quo" you were alluding to.

As long as master of superstring remains intended for current Pulsar master, and we PR changes intended for current Pulsar master promptly (or manually test such things before merging here?) then that should cover it.

Otherwise, I agree, efforts to have this done in CI would be worthwhile. We have a thing called action-pulsar-dependency (https://github.com/pulsar-edit/action-pulsar-dependency), unclear if just throwing a uses: pulsar-edit/[email protected] into a workflow would take care of our testing needs? See this usage example for action-pulsar-dependency, it's being use already in our github package fork... https://github.com/pulsar-edit/github/blob/5c1590c4537b601d0743bd491c8ac58d5c823856/.github/workflows/ci.yml#L29