Add --main-process flag to run specs in the main process #11826
Conversation
…so that we can write unit tests for code in the main process. Below a list of conventions we'll be adopting: 1. Use mocha as the test framework. 2. Use chai as the assertion library. 3. Name specs `foo-bar.spec.js`, to prevent the renderer process from running specs written fro the main process e.g. when calling `atom --test spec`. 4. Write specs in ES6. Although somewhat inconsistent with the conventions we use for renderer process specs, this will hopefully be a first step towards migrating our entire Jasmine 1.3 test suite to a more modern environment.
|
/cc: @atom/core for 👀, especially for 💭 about using a test framework that's not Jasmine for main process specs. Also, in a later pull-request, I'd like to move all the renderer production code and specs under a |
| }, | ||
| "devDependencies": { | ||
| "chai": "^3.5.0", | ||
| "mocha": "^2.5.1" |
There was a problem hiding this comment.
Should these go in build/package.json's dependencies?
There was a problem hiding this comment.
Good question! 👍
I think Atom needs those dependencies at runtime, so we should just probably put them under the root's package.json as we do for jasmine. On the other hand, package authors won't be able to use it anyways, so it's not really useful to include it in the final executable (although it seems like even when in devDependencies, those packages are included in the final asar archive 😕).
I'll put those two dependencies under the regular dependencies (to be consistent with jasmine) and we can refine this later if needed.
|
I'm all for everything about this. Giant 👍 from me. Does this still load the spec helpers for main process tests? If so, anything we should change about that? And if not, anything from in there we should steal? :) |
| runMainProcessSpecs = (callback) -> | ||
| appPath = getAppPath() | ||
| resourcePath = process.cwd() | ||
| mainProcessSpecsPath = path.resolve('spec/browser') |
There was a problem hiding this comment.
is there a reason you chose to go with spec/browser rather than spec/main? IMO main is a bit more clear, and it looks like you use it almost everywhere else in the code
There was a problem hiding this comment.
The main reason was that we store main process code under src/browser, so I thought I'd name the spec folder accordingly. Also main sounds a bit generic, so I guess we could rename the browser folders to main-process and have everything named consistently.
Thanks, @kuychaco!
|
All this sounds great, thanks for doing this @as-cii ✨ |
I'd like to keep this super simple for now, and add helpers as needed. We are probably not going to need most of that code anyways and I feel like we should copy it later if there's something handy in there that we can steal. I'll go ahead and 🚢 this as soon as the build gets green, so that we can use it on #11828. Thanks for great review, @BinaryMuse and @kuychaco! 🙏 |
…so that we can write unit tests for code in the main process. Below a list of conventions we'll be adopting:
foo-bar.spec.js, to prevent the renderer process from running specs written for the main process e.g. when callingatom --test spec.spec/browser.Although somewhat inconsistent with the conventions we use for renderer process specs, this will hopefully be a first step towards migrating our entire Jasmine 1.3 test suite to a more modern environment.