Skip to content

(feat) added argument for karma configuration file #4564

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

Merged
merged 7 commits into from
Feb 10, 2017
Merged

(feat) added argument for karma configuration file #4564

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3e7b167
(feat) added argument for karma configuration file
Feb 9, 2017
11da535
(e2e) added test + (bug) fixed type
Feb 9, 2017
d9e064a
(update) changed config-file flag to config for consistency
Feb 9, 2017
79c674b
Merge branch 'master' into master
Feb 10, 2017
d4662cf
(fix) updated function name
Feb 10, 2017
01e7fef
(update) requested changes
Feb 10, 2017
5e98d7e
Merge branch 'master' into master
Feb 10, 2017
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
2 changes: 2 additions & 0 deletions packages/@angular/cli/commands/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ export interface TestOptions {
build?: boolean;
sourcemap?: boolean;
progress?: boolean;
config: string;
}


const TestCommand = EmberTestCommand.extend({
availableOptions: [
{ name: 'watch', type: Boolean, default: true, aliases: ['w'] },
{ name: 'code-coverage', type: Boolean, default: false, aliases: ['cc'] },
{ name: 'config', type: String, aliases: ['c', 'cf'] },
Copy link
Contributor

Choose a reason for hiding this comment

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

Use just the c alias instead. I understand the old command had both, but we don't use it so no user would know and this way we don't use up another possible alias.

Copy link
Author

Choose a reason for hiding this comment

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

Updated with your comments. Though, 'cc' and 'c' for respectively code coverage and config is little bit scary, no ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's ok enough. It's true that -c -cc is kinda weird, but I'd expect -c to be used much more than -cc, and less so them together. And when users use them together, they shouldn't have to change -c to -cf.

Looking at the PoV of making mistakes, since before both -c and -cf were allowed then you could just as easily mistakenly do -cc instead if you were used to -c.

{ name: 'single-run', type: Boolean, default: false, aliases: ['sr'] },
{ name: 'progress', type: Boolean, default: true},
{ name: 'browsers', type: String },
Expand Down
2 changes: 1 addition & 1 deletion packages/@angular/cli/ember-cli/lib/commands/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = Command.extend({

availableOptions: [
{ name: 'environment', type: String, default: 'test', aliases: ['e'] },
{ name: 'config-file', type: String, aliases: ['c', 'cf']},
{ name: 'config', type: String, aliases: ['c', 'cf']},
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to change this, our command overrides this one.

{ name: 'server', type: Boolean, default: false, aliases: ['s'] },
{ name: 'host', type: String, aliases: ['H'] },
{ name: 'test-port', type: Number, default: defaultPort, aliases: ['tp'], description: 'The test port to use when running with --server.' },
Expand Down
3 changes: 2 additions & 1 deletion packages/@angular/cli/tasks/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export default Task.extend({
const projectRoot = this.project.root;
return new Promise((resolve) => {
const karma = requireProjectModule(projectRoot, 'karma');
const karmaConfig = path.join(projectRoot, this.project.ngConfig.config.test.karma.config);
const karmaConfig = path.join(projectRoot, options.config ||
this.project.ngConfig.config.test.karma.config);

let karmaOptions: any = Object.assign({}, options);

Expand Down
5 changes: 4 additions & 1 deletion tests/e2e/tests/test/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { ng } from '../../utils/process';
import { copyFile } from '../../utils/fs';

export default function () {
// make sure both --watch=false and --single-run work
return ng('test', '--single-run')
.then(() => ng('test', '--watch=false'));
.then(() => ng('test', '--watch=false'))
.then(() => copyFile('./karma.conf.js', './karma.conf.bis.js'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Use moveFile instead of copy please, otherwise the feature might be broken and we don't know since the old file is used.

The test harness will take care of moving it back after the test.

.then(() => ng('test', '--single-run', '--config', 'karma.conf.bis.js'));
}