Update Grunt task #813
Update Grunt task #813
Conversation
* Allow grunt options to specify 'config' to point to a config file and/or section * Change node/util.getConfig to accept an optional second argument to allow passing in other command line arguments * Updated task tests Fixes theintern#812
Codecov Report
@@ Coverage Diff @@
## master #813 +/- ##
==========================================
+ Coverage 93.02% 93.04% +0.01%
==========================================
Files 54 54
Lines 4347 4358 +11
Branches 943 945 +2
==========================================
+ Hits 4044 4055 +11
Misses 303 303
Continue to review full report at Codecov.
|
| @@ -48,15 +48,15 @@ export function expandFiles(patterns?: string[] | string) { | |||
| * Get the user-supplied config data, which may include command line args and a | |||
| * config file. | |||
| */ | |||
| export function getConfig(file?: string) { | |||
| export function getConfig(file?: string, argv: string[] = process.argv) { | |||
There was a problem hiding this comment.
We shouldn't need the string[] type since it can be inferred from the default arg. Also, should we have an overload that takes just an argv?
There was a problem hiding this comment.
It'd also be nice to add typedoc @param comments so we can point out that argv args should at position 2.
|
|
||
| interface TaskOptions extends grunt.task.ITaskOptions, Partial<Config> { | ||
| [key: string]: any; | ||
| } | ||
|
|
||
| function configure(options: TaskOptions): Promise<{ |
There was a problem hiding this comment.
configure is a weird name for this since its not configuring anything (although I don't suppose it matters since its internal). Also, Intern code typically has things like module-level constants and functions declare below exports.
| }> { | ||
| if (options.config) { | ||
| return getConfig(options.config, []).then(({ config }) => { | ||
| delete options.config; |
There was a problem hiding this comment.
Instead of deleting config from the passed-in options, it'd be nicer to return a new object that didn't have the config property (since we're returning options anyway).
| @@ -46,15 +55,15 @@ registerSuite('tasks/intern', function() { | |||
| }, | |||
|
|
|||
| beforeEach() { | |||
| mockRun.reset(); | |||
| sandbox.reset(); | |||
| mockRun.resolves(); | |||
There was a problem hiding this comment.
If we made mockRun = sandbox.stub().resolves(), could we get buy with just calling sandbox.resetHistory() above (instead of sandbox.reset())?
There was a problem hiding this comment.
There are a few other stubs I've set up that need to be reset every time
|
Merged in def6346 |
Fixes #812