Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Grunt task #813

Closed
wants to merge 1 commit into from
Closed

Update Grunt task #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/lib/node/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd also be nice to add typedoc @param comments so we can point out that argv args should at position 2.

let args: { [key: string]: any } = {};

if (process.env['INTERN_ARGS']) {
mixin(args, parseArgs(parse(process.env['INTERN_ARGS'])));
}

if (process.argv.length > 2) {
mixin(args, parseArgs(process.argv.slice(2)));
if (argv.length > 2) {
mixin(args, parseArgs(argv.slice(2)));
}

if (file) {
Expand Down
30 changes: 25 additions & 5 deletions src/tasks/intern.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,46 @@
import global from '@dojo/shim/global';
import { assign } from '@dojo/core/lang';

import Node from '../lib/executors/Node';
import { Config } from '../lib/executors/Executor';
import { getConfig } from '../lib/node/util';

interface TaskOptions extends grunt.task.ITaskOptions, Partial<Config> {
[key: string]: any;
}

function configure(options: TaskOptions): Promise<{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

config: Partial<Config>;
options: TaskOptions;
}> {
if (options.config) {
return getConfig(options.config, []).then(({ config }) => {
delete options.config;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


return { config, options };
});
}

return Promise.resolve({ config: {}, options });
}

export = function(grunt: IGrunt) {
grunt.registerMultiTask('intern', function() {
const done = this.async();
const config = this.options<TaskOptions>({});
const options = assign({}, this.options<TaskOptions>({}));

// Force colored output for istanbul report
process.env.FORCE_COLOR = 'true';

const intern = (global.intern = new Node(config));
intern.run().then(finish, finish);
configure(options).then(({ config, options }) => {
const intern = (global.intern = new Node());
intern.configure(config);
intern.configure(options);

return intern.run();
}).then(finish, finish);

function finish(error?: any) {
// Remove the global intern object when we're done; this will allow
// Intern to be run again in the same process
global.intern = null;
done(error);
}
Expand Down
174 changes: 119 additions & 55 deletions tests/unit/tasks/intern.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,36 @@
import { spy, stub } from 'sinon';
import * as sinon from 'sinon';
import _gruntTask from 'src/tasks/intern';

const mockRequire = intern.getPlugin<mocking.MockRequire>('mockRequire');

registerSuite('tasks/intern', function() {
const mockDone = stub();
const sandbox = sinon.sandbox.create();

let mockDone: sinon.SinonSpy;

function setupDone() {
return new Promise(resolve => {
mockDone = sinon.spy(() => resolve());
});
}

const mockGrunt = {
registerMultiTask: spy(() => {}),
async: spy(() => mockDone),
options: spy(() => {
return { foo: 'bar' };
})
registerMultiTask: sandbox.stub(),
async: sandbox.spy(() => mockDone),
options: sandbox.stub()
};

const mockRun = stub();
const mockRun = sandbox.stub();
const mockConfigure = sandbox.stub();

const mockGetConfig = sandbox.stub();
class MockNode {
config: any;
run: Function;
constructor(config: any) {
configure: Function;
constructor() {
executors.push(this);
this.config = config;
this.run = mockRun;
this.configure = mockConfigure;
}
}

Expand All @@ -34,7 +42,8 @@ registerSuite('tasks/intern', function() {
before() {
return mockRequire(require, 'src/tasks/intern', {
'src/lib/executors/Node': { default: MockNode },
'@dojo/shim/global': { default: {} }
'@dojo/shim/global': { default: {} },
'src/lib/node/util': { getConfig: mockGetConfig }
}).then(handle => {
removeMocks = handle.remove;
gruntTask = handle.module;
Expand All @@ -46,15 +55,15 @@ registerSuite('tasks/intern', function() {
},

beforeEach() {
mockRun.reset();
sandbox.reset();
mockRun.resolves();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we made mockRun = sandbox.stub().resolves(), could we get buy with just calling sandbox.resetHistory() above (instead of sandbox.reset())?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few other stubs I've set up that need to be reset every time

mockDone.reset();
executors = [];
},

tests: {
'task registration'() {
gruntTask(<any>mockGrunt);

assert.equal(
mockGrunt.registerMultiTask.callCount,
1,
Expand All @@ -67,55 +76,110 @@ registerSuite('tasks/intern', function() {
);
},

'run task'() {
const dfd = this.async();
gruntTask(<any>mockGrunt);
const callback = mockGrunt.registerMultiTask.getCall(0).args[1];
callback.call(mockGrunt);
assert.lengthOf(
executors,
1,
'1 executor should have been created'
);
assert.equal(
mockRun.callCount,
1,
'intern should have been run'
);
assert.deepEqual(executors[0].config, { foo: 'bar' });
setTimeout(
dfd.callback(() => {
'run task': {
'config'() {
mockGrunt.registerMultiTask.callsArgOn(1, mockGrunt);
mockGrunt.options.returns({
config: '@coverage',
foo: 'bar'
});
mockGetConfig.resolves({
config: {
spam: 'ham'
}
});
const done = setupDone();

gruntTask(<any>mockGrunt);

return done.then(() => {
assert.lengthOf(
executors,
1,
'1 executor should have been created'
);
assert.equal(
mockRun.callCount,
1,
'intern should have been run'
);
assert.equal(mockGetConfig.callCount, 1);
assert.equal(
mockGetConfig.getCall(0).args[0],
'@coverage'
);
assert.equal(mockConfigure.callCount, 2);
assert.deepEqual(mockConfigure.getCall(0).args[0], {
spam: 'ham'
});
assert.deepEqual(mockConfigure.getCall(1).args[0], {
foo: 'bar'
});
assert.equal(mockDone.callCount, 1);
// First arg is an error, so it should be undefined here
assert.isUndefined(mockDone.getCall(0).args[0]);
})
);
});
},

'no config'() {
mockGrunt.registerMultiTask.callsArgOn(1, mockGrunt);
mockGrunt.options.returns({
foo: 'bar'
});
const done = setupDone();

gruntTask(<any>mockGrunt);

return done.then(() => {
assert.lengthOf(
executors,
1,
'1 executor should have been created'
);
assert.equal(
mockRun.callCount,
1,
'intern should have been run'
);
assert.equal(mockGetConfig.callCount, 0);
assert.equal(mockConfigure.callCount, 2);
assert.deepEqual(mockConfigure.getCall(0).args[0], {});
assert.deepEqual(mockConfigure.getCall(1).args[0], {
foo: 'bar'
});
assert.equal(mockDone.callCount, 1);
// First arg is an error, so it should be undefined here
assert.isUndefined(mockDone.getCall(0).args[0]);
});
}
},

error() {
const dfd = this.async();
gruntTask(<any>mockGrunt);
const callback = mockGrunt.registerMultiTask.getCall(0).args[1];
mockGrunt.registerMultiTask.callsArgOn(1, mockGrunt);
mockGrunt.options.returns({
foo: 'bar'
});
const error = new Error('bad');
mockRun.rejects(error);
callback.call(mockGrunt);
assert.lengthOf(
executors,
1,
'1 executor should have been created'
);
assert.equal(
mockRun.callCount,
1,
'intern should have been run'
);
assert.deepEqual(executors[0].config, { foo: 'bar' });
setTimeout(
dfd.callback(() => {
assert.equal(mockDone.callCount, 1);
assert.equal(mockDone.getCall(0).args[0], error);
})
);

const done = setupDone();

gruntTask(<any>mockGrunt);

return done.then(() => {
assert.lengthOf(
executors,
1,
'1 executor should have been created'
);
assert.equal(
mockRun.callCount,
1,
'intern should have been run'
);
assert.equal(mockDone.callCount, 1);
assert.equal(mockDone.getCall(0).args[0], error);
});
}
}
};
Expand Down