Add significantly more test coverage and various bug/doc fixes. #85
Conversation
ryan-roemer
force-pushed
the
chore-more-tests
branch
3 times, most recently
from
d86f11d
to
598bbb7
Compare
| @@ -38,6 +38,11 @@ var Config = module.exports = function (cfg) { | |||
| this.scripts = this._loadScripts(this.archetypes); | |||
| }; | |||
|
|
|||
| // Expose `require()` for testing. | |||
There was a problem hiding this comment.
I understand why this is done after reading your tests, but can you elaborate in this comment a little more? On first read it's very unclear why this additional logic is here. Just mentioning something like "require needs to be stubbed to work with mock filesystem in tests" would be helpful.
There was a problem hiding this comment.
I've got a more full explanation here -- https://github.com/FormidableLabs/builder/pull/85/files#diff-4ffc5fe4c4f93897ccc6fb2331ae5e44R56
Basically (1) fs mocking doesn't work with Node 4+, and (2) you want to defeat the require cache is what we mutate in the actual tests for our use cases.
But this code is mostly here to just allow the tests to decide how to override / stub...
There was a problem hiding this comment.
I understand why this is being done, I was just suggesting a tiny bit more explanation in the comment in this file.
Implementation work for #9 * Add comprehensive "almost" functional tests in `builder-core.spec.js` using `mock-fs` and a ton of actual usage scenarios. * Capture JSON parsing errors for `builder envs` better. * Fix bug wherein `builder --version` displayed help instead of version. * Refactor Task to take `runner` option to avoid circular dependencies. * Refactor `--setup` execution model to resolve and use a `Task` object straight up rather than a `builder run <setup-task-name>`. * Refactor `config.js` and `log.js` to have stubable methods in tests. * Enhance `builder-core` to take an options argument. * Fix minor README.md typo.
ryan-roemer
force-pushed
the
chore-more-tests
branch
from
21c299d
to
a652651
Compare
|
Post-merge review welcome! |
Add significantly more test coverage and various bug/doc fixes.
Implementation work for #9
builder-core.spec.jsusingmock-fsand aton of actual usage scenarios.
builder envsbetter.builder --versiondisplayed help instead of version.--setupexecution model to resolve and use aTaskobject straight uprather than a
builder run <setup-task-name>.config.jsandlog.jsto have stubable methods in tests.builder-coreto take an options argument./cc @chaseadamsio @benbayard @baer @coopy @zachhale