Skip to content

added debug argument to ng test task. #1799

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.

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

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion packages/angular-cli/blueprints/ng2/files/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module.exports = function (config) {
config: './angular-cli.json',
environment: 'dev'
},
reporters: ['progress', 'karma-remap-istanbul'],
reporters: config.debug ? ['progress'] : ['progress', 'karma-remap-istanbul'],
port: 9876,
colors: true,
logLevel: config.LOG_INFO,
Expand Down
1 change: 1 addition & 0 deletions packages/angular-cli/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {CliConfig} from '../models/config';
const NgCliTestCommand = TestCommand.extend({
availableOptions: [
{ name: 'watch', type: Boolean, default: true, aliases: ['w'] },
{ name: 'debug', type: Boolean, default: false, aliases: ['d'] },
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this option being passed into the karma config? I don't think it's being passed in yet.

My suggestion is this: in karma.conf.js add in the debug property to the angularCli options:

    angularCli: {
      config: './angular-cli.json',
      environment: 'dev',
      debug: false
    },

Then you'll need to also change packages/angular-cli/tasks/test.ts to set this to true in the options variable that's being passed into karma.Server.

Make sure it works though, this is the kind of thing I don't know we can really do CI tests without doing really weird stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i actually implemented debug false as default so it doesn't need to be passed. i added an argument to the test command --debug (-d). I think we would add a npm script npm run test:debug which would call ng test --debug

Copy link
Contributor

Choose a reason for hiding this comment

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

I was tracing back the code and realize you are right. Since we just start karma with all the options that the command receives, the debug property will be there. My bad!

{ name: 'browsers', type: String },
{ name: 'colors', type: Boolean },
{ name: 'log-level', type: String },
Expand Down
5 changes: 3 additions & 2 deletions packages/angular-cli/models/webpack-build-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
const path = require('path');
const webpack = require('webpack');

const getWebpackTestConfig = function (projectRoot, environment, appConfig) {
const getWebpackTestConfig = function (projectRoot, environment, appConfig, debug) {

const appRoot = path.resolve(projectRoot, appConfig.root);
var debug = typeof debug !== 'undefined' ? debug : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Linting says this variable is being shadowed.


return {
devtool: 'inline-source-map',
Expand Down Expand Up @@ -66,7 +67,7 @@ const getWebpackTestConfig = function (projectRoot, environment, appConfig) {
{ test: /\.(jpg|png)$/, loader: 'url-loader?limit=128000' },
{ test: /\.html$/, loader: 'raw-loader', exclude: [path.resolve(appRoot, appConfig.index)] }
],
postLoaders: [
postLoaders: debug ? [] : [
Copy link
Contributor

Choose a reason for hiding this comment

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

In the latest webpack update, the loader configs changed a bit and is now a single rules array.

You're probably going to have to add the sourcemap-istanbul-instrumenter-loader conditionally, but by itself, at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok @filipesilva . i will update this branch to the latest changes in master and fix the linting issue you mentioned.

{
test: /\.(js|ts)$/, loader: 'sourcemap-istanbul-instrumenter-loader',
exclude: [
Expand Down
5 changes: 2 additions & 3 deletions packages/angular-cli/plugins/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const init = (config) => {
const environment = config.angularCli.environment || 'dev';

// add webpack config
const webpackConfig = getWebpackTestConfig(config.basePath, environment, appConfig);
config.webpack = getWebpackTestConfig(config.basePath, environment, appConfig, config.debug);
const webpackMiddlewareConfig = {
noInfo: true, // Hide webpack output because its noisy.
stats: { // Also prevent chunk and module display output, cleaner look. Only emit errors.
Expand All @@ -25,7 +25,6 @@ const init = (config) => {
chunkModules: false
}
};
config.webpack = Object.assign(webpackConfig, config.webpack);
config.webpackMiddleware = Object.assign(webpackMiddlewareConfig, config.webpackMiddleware);

// replace the angular-cli preprocessor with webpack+sourcemap
Expand All @@ -45,4 +44,4 @@ preprocessor.$inject = []
module.exports = Object.assign({
'framework:angular-cli': ['factory', init],
'preprocessor:angular-cli': ['factory', preprocessor]
}, require('karma-webpack'), require('karma-sourcemap-loader'));
}, require('karma-webpack'), require('karma-sourcemap-loader'));