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.

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

feat(logging): Allow log to a file #954

Merged
merged 38 commits into from
Jul 17, 2018
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
01a9b36
feat(Logging api): add logging api
nicojs May 13, 2018
187855e
feat(logging-api): first usage of logging api
nicojs May 15, 2018
a6feb8c
Merge branch 'master' into 748-logging-api
nicojs May 26, 2018
8e50b4f
Update log4js 2.7
nicojs May 27, 2018
09eb2f2
Merge branch 'master' into 748-logging-api
nicojs Jul 1, 2018
c50d69d
feat: add file log level option to stryker config
nicojs Jul 6, 2018
98c2bfb
chore: add stryker.log to gitignore
nicojs Jul 6, 2018
c002d5c
docs(stryker): Document new file log level option
nicojs Jul 6, 2018
7d2f3df
feat(file logging): add logging server
nicojs Jul 6, 2018
6590624
refactor: remove unused import
nicojs Jul 6, 2018
691e3a7
test(logging): Add unit test for multi appender
nicojs Jul 6, 2018
042117e
refactor(stryker): Add semicolons
nicojs Jul 6, 2018
188bada
test(logging): Add unit tests for the log configurator
nicojs Jul 6, 2018
d5d483e
Merge branch 'master' into 748-logging-api
nicojs Jul 6, 2018
4fdc250
fix(karma-runner): remove logging logic from karma
nicojs Jul 6, 2018
1729794
build(dev depedencies): remove unused types for old log4js
nicojs Jul 6, 2018
c14a520
chore(karma-runner): Add express.js types back
nicojs Jul 6, 2018
0a30c08
test(stryker-api): fix failing test
nicojs Jul 6, 2018
5a92cc3
test(logging): Remove accidental `.only`
nicojs Jul 6, 2018
be5477f
fix(logging): Remove color from file layout
nicojs Jul 6, 2018
3339086
test(logging): Add logging integration test
nicojs Jul 6, 2018
10bed57
refactor(ConfigReader): don't use `process.exit`
nicojs Jul 6, 2018
a820577
fix(taskkill): make killing of tasks more relient on windows
nicojs Jul 6, 2018
ee1efda
test(api): Improve error logging on integration test
nicojs Jul 7, 2018
33aa4d9
test(karma-runner): Fix failing test in karma runner after merge
nicojs Jul 7, 2018
c2882b9
fix(logging): Make logging server more robust against race conditions
nicojs Jul 7, 2018
f59d0c2
test(stryker): Higher timeout for integration tests
nicojs Jul 7, 2018
9d6a4f6
refactor: Remove code responsible for retrying different ports as it …
nicojs Jul 7, 2018
82db388
Merge branch 'master' into 748-logging-api
nicojs Jul 12, 2018
145e583
feat(jest-runner): Use new logging API
nicojs Jul 12, 2018
d10b523
build(karma-runner): Add express types back again
nicojs Jul 12, 2018
cbe94cc
refactor(logging): Implement review comments
nicojs Jul 12, 2018
18859d6
Merge branch 'master' into 748-logging-api
nicojs Jul 14, 2018
0761b79
fix(stryker-api): Update stryker-api peer dependency in preparation o…
nicojs Jul 14, 2018
07cc2b1
refactor(isolated-runner): Remove `IsolatedRunnerOptions`
nicojs Jul 17, 2018
bafa8e8
Merge branch 'master' into 748-logging-api
nicojs Jul 17, 2018
256f8ab
refactor: add semicolon
nicojs Jul 17, 2018
6f64f1d
fix(log4js): update to log4js 3
nicojs Jul 17, 2018
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
node_modules
lerna-debug.log
npm-debug.log
stryker.log

coverage
reports
Expand Down
2 changes: 1 addition & 1 deletion integrationTest/test/jasmine-jasmine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"description": "A module to perform an integration test",
"main": "index.js",
"scripts": {
"pretest": "rimraf \"reports\" \"verify/*.map\" && tsc -p .",
"pretest": "rimraf \"reports\" \"verify/*.map\" \"stryker.log\"",
"test": "stryker run",
"posttest": "mocha --require ts-node/register verify/*.ts"
},
Expand Down
6 changes: 4 additions & 2 deletions integrationTest/test/jasmine-jasmine/stryker.conf.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { LogLevel } = require('stryker-api/core');
module.exports = function (config) {
config.set({
mutate: ['lib/**/*.js'],
Expand All @@ -6,7 +7,8 @@ module.exports = function (config) {
testFramework: 'jasmine',
testRunner: 'jasmine',
reporter: ['clear-text', 'event-recorder'],
maxConcurrentTestRunners: 2,
jasmineConfigFile: 'spec/support/jasmine.json'
maxConcurrentTestRunners: 1,
jasmineConfigFile: 'spec/support/jasmine.json',
fileLogLevel: LogLevel.Debug
});
};
7 changes: 7 additions & 0 deletions integrationTest/test/jasmine-jasmine/verify/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,11 @@ describe('After running stryker with test runner jasmine, test framework jasmine
expect(scoreResult.noCoverage).eq(1);
expect(scoreResult.mutationScore).greaterThan(85).and.lessThan(86);
});

it('should write to a log file', async () => {
const strykerLog = await fs.readFile('./stryker.log', 'utf8');
expect(strykerLog).contains('INFO InputFileResolver Found 2 of 10 file(s) to be mutated');
expect(strykerLog).matches(/Stryker Done in \d+ seconds/);
// expect(strykerLog).not.contains('ERROR'); TODO
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in a todo?

Copy link
Member Author

Choose a reason for hiding this comment

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

We get an error after 10 test runs because of this issue: jasmine/jasmine-npm#134. I'll add it as a comment.

});
});
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"@types/istanbul": "^0.4.29",
"@types/karma": "^1.7.4",
"@types/lodash": "^4.14.110",
"@types/log4js": "0.0.32",
"@types/mkdirp": "^0.5.2",
"@types/mocha": "^2.2.44",
"@types/mz": "0.0.32",
Expand Down
3 changes: 2 additions & 1 deletion packages/stryker-api/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export { default as Position } from './src/core/Position';
export { default as Location } from './src/core/Location';
export { default as Range } from './src/core/Range';
export { default as MutatorDescriptor } from './src/core/MutatorDescriptor';
export { default as MutationScoreThresholds } from './src/core/MutationScoreThresholds';
export { default as MutationScoreThresholds } from './src/core/MutationScoreThresholds';
export { default as LogLevel } from './src/core/LogLevel';
4 changes: 4 additions & 0 deletions packages/stryker-api/logging.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export { default as LoggerFactory } from './src/logging/LoggerFactory';
export { default as Logger } from './src/logging/Logger';
export { default as LoggerFactoryMethod } from './src/logging/LoggerFactoryMethod';
export { default as getLogger } from './src/logging/getLogger';
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need getLogger in a separate file or can we also just place the content here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is how our entire API is build up right now. Everything in separate files. getLogger is also used from LoggerFactoryMethod and others. You want me to move the entire logging API to this file?

Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it like this for now.

5 changes: 3 additions & 2 deletions packages/stryker-api/src/config/Config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StrykerOptions, MutatorDescriptor, MutationScoreThresholds } from '../../core';
import { LogLevel, StrykerOptions, MutatorDescriptor, MutationScoreThresholds } from '../../core';

export default class Config implements StrykerOptions {

Expand All @@ -7,7 +7,8 @@ export default class Config implements StrykerOptions {
files: string[];
mutate: string[];

logLevel = 'info';
logLevel: LogLevel = LogLevel.Information;
fileLogLevel: LogLevel = LogLevel.Off;
timeoutMs = 5000;
timeoutFactor = 1.5;
plugins: string[] = ['stryker-*'];
Expand Down
12 changes: 12 additions & 0 deletions packages/stryker-api/src/core/LogLevel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

enum LogLevel {
Off = 'off',
Fatal = 'fatal',
Error = 'error',
Warning = 'warn',
Information = 'info',
Debug = 'debug',
Trace = 'trace'
}

export default LogLevel;
43 changes: 25 additions & 18 deletions packages/stryker-api/src/core/StrykerOptions.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
import MutationScoreThresholds from './MutationScoreThresholds';
import MutatorDescriptor from './MutatorDescriptor';
import LogLevel from './LogLevel';

interface StrykerOptions {
// this ensures that custom config for for example 'karma' can be added under the 'karma' key
// this ensures that plugins can load custom config.
[customConfig: string]: any;

/**
* The files array determines which files are in scope for mutation testing.
* These include library files, test files and files to mutate, but should NOT include test framework files (for example jasmine).
* Each element can be either a string or an object with 2 properties
* * `string`: A globbing expression used for selecting the files needed to run the tests.
* * { pattern: 'pattern', included: true, mutated: false, transpiled: true }:
* * The `pattern` property is mandatory and contains the globbing expression used for selecting the files
* * The `included` property is optional and determines whether or not this file should be loaded initially by the test-runner (default: true)
* * The `mutated` property is optional and determines whether or not this file should be targeted for mutations (default: false)
* * The `transpiled` property is optional and determines whether or not this file should be transpiled by a transpiler (see `transpilers` config option) (default: true)
*
* @example
* files: ['test/helpers/**\/*.js', 'test/unit/**\/*.js', { pattern: 'src/**\/*.js', included: false }],
* A list of globbing expression used for selecting the files that should be mutated.
*/
files?: string[];
mutate?: string[];

/**
* A list of globbing expression used for selecting the files that should be mutated.
* With `files` you can choose which files should be included in your test runner sandbox.
* This is normally not needed as it defaults to all files not ignored by git.
* Try it out yourself with this command: `git ls-files --others --exclude-standard --cached --exclude .stryker-tmp`.
*
* If you do need to override `files` (for example: when your project does not live in a git repository),
* you can override the files here.
*
* When using the command line, the list can only contain a comma separated list of globbing expressions.
* When using the config file you can provide an array with `string`s
*
* You can *ignore* files by adding an exclamation mark (`!`) at the start of an expression.
*
*/
mutate?: string[];
files?: string[];

/**
* Specify the maximum number of concurrent test runners. Useful if you don't want to use
Expand Down Expand Up @@ -104,9 +105,15 @@ interface StrykerOptions {
reporter?: string | string[];

/**
* The log4js log level. Possible values: fatal, error, warn, info, debug, trace, all and off. Default is "info"
* The log level for logging to a file. If defined, stryker will output a log file called "stryker.log".
* Default: "off"
*/
fileLogLevel?: LogLevel;

/**
* The log level for logging to the console. Default: "info".
*/
logLevel?: string;
logLevel?: LogLevel;

/**
* Indicates whether or not to symlink the node_modules folder inside the sandbox folder(s).
Expand Down
15 changes: 15 additions & 0 deletions packages/stryker-api/src/logging/Logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export default interface Logger {
isTraceEnabled(): boolean;
isDebugEnabled(): boolean;
isInfoEnabled(): boolean;
isWarnEnabled(): boolean;
isErrorEnabled(): boolean;
isFatalEnabled(): boolean;

trace(message: string, ...args: any[]): void;
debug(message: string, ...args: any[]): void;
info(message: string, ...args: any[]): void;
warn(message: string, ...args: any[]): void;
error(message: string, ...args: any[]): void;
fatal(message: string, ...args: any[]): void;
}
30 changes: 30 additions & 0 deletions packages/stryker-api/src/logging/LoggerFactory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import LoggerFactoryMethod from './LoggerFactoryMethod';
import Logger from './Logger';

const noopLogger: Logger = {
isTraceEnabled(): boolean { return false; },
isDebugEnabled(): boolean { return false; },
isInfoEnabled(): boolean { return false; },
isWarnEnabled(): boolean { return false; },
isErrorEnabled(): boolean { return false; },
isFatalEnabled(): boolean { return false; },
trace(): void { },
debug(): void { },
info(): void { },
warn(): void { },
error(): void { },
fatal(): void { }
};

let logImplementation: LoggerFactoryMethod = () => noopLogger;

export default class LoggerFactory {

static setLogImplementation(implementation: LoggerFactoryMethod) {
logImplementation = implementation;
}

static getLogger: LoggerFactoryMethod = (categoryName?: string) => {
return logImplementation(categoryName);
}
}
12 changes: 12 additions & 0 deletions packages/stryker-api/src/logging/LoggerFactoryMethod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Logger from './Logger';

/**
* Get a logger instance. Instance is cached on categoryName level.
*
* @param {String} [categoryName] name of category to log to.
* @returns {Logger} instance of logger for the category
* @static
*/
export default interface LoggerFactoryMethod {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this exists and what it does

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the description to:

* Represents a factory to get loggers by category name.
 * This interface is used to describe the shape of a logger factory method.

Is this better?

(categoryName?: string): Logger;
}
6 changes: 6 additions & 0 deletions packages/stryker-api/src/logging/getLogger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import LoggerFactory from './LoggerFactory';
import LoggerFactoryMethod from './LoggerFactoryMethod';

const getLogger: LoggerFactoryMethod = LoggerFactory.getLogger;

export default getLogger;
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,20 @@ describe('we have a module using stryker', function () {

const modulePath = path.resolve(__dirname, '../../../testResources/module');

const execInModule = (command: string) => {
console.log(`Exec '${command}' in ${modulePath}`);
return exec(command, { cwd: modulePath });
};
function execInModule (command: string): Promise<[string, string]> {
return new Promise((res, rej) => {
console.log(`Exec '${command}' in ${modulePath}`);
exec(command, { cwd: modulePath }, (error, stdout, stderr) => {
if (error) {
console.log(stdout);
console.error(stderr);
rej(error);
} else {
res([stdout, stderr]);
}
});
});
}

describe('after installing Stryker', () => {

Expand Down
19 changes: 19 additions & 0 deletions packages/stryker-api/test/unit/core/LogLevelSpec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { expect } from 'chai';
import LogLevel from '../../../src/core/LogLevel';

describe('LogLevel', () => {

function arrangeActAssertLogLevel(actual: LogLevel, expected: string) {
it(`should provide "${expected}" for log level "${actual}"`, () => {
expect(actual).eq(expected);
});
}

arrangeActAssertLogLevel(LogLevel.Off, 'off');
arrangeActAssertLogLevel(LogLevel.Fatal, 'fatal');
arrangeActAssertLogLevel(LogLevel.Error, 'error');
arrangeActAssertLogLevel(LogLevel.Warning, 'warn');
arrangeActAssertLogLevel(LogLevel.Information, 'info');
arrangeActAssertLogLevel(LogLevel.Debug, 'debug');
arrangeActAssertLogLevel(LogLevel.Trace, 'trace');
});
4 changes: 2 additions & 2 deletions packages/stryker-api/testResources/module/useCore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StrykerOptions, Factory, File, Position, Location, Range } from 'stryker-api/core';
import { StrykerOptions, Factory, File, Position, Location, Range, LogLevel } from 'stryker-api/core';

const options: StrykerOptions = {};
const optionsAllArgs: StrykerOptions = {
Expand All @@ -8,7 +8,7 @@ const optionsAllArgs: StrykerOptions = {
testFramework: 'string',
testRunner: 'string',
reporter: 'string',
logLevel: 'string',
logLevel: LogLevel.Fatal,
timeoutMs: 1,
timeoutFactor: 2,
plugins: ['string'],
Expand Down
1 change: 0 additions & 1 deletion packages/stryker-babel-transpiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"license": "Apache-2.0",
"dependencies": {
"babel-core": "6.26.3",
"log4js": "^1.1.1",
"minimatch": "^3.0.4"
},
"devDependencies": {
Expand Down
3 changes: 1 addition & 2 deletions packages/stryker-babel-transpiler/src/BabelConfigReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ import * as fs from 'fs';
import * as path from 'path';
import { Config } from 'stryker-api/config';
import { CONFIG_KEY_FILE, CONFIG_KEY_OPTIONS } from './helpers/keys';
import { getLogger, setGlobalLogLevel } from 'log4js';
import { getLogger } from 'stryker-api/logging';

export default class BabelConfigReader {
private readonly log = getLogger(BabelConfigReader.name);

public readConfig(config: Config): babel.TransformOptions {
setGlobalLogLevel(config.logLevel);
const babelConfig: babel.TransformOptions = config[CONFIG_KEY_OPTIONS] || this.getConfigFile(config) || {};
this.log.debug(`babel config is: ${JSON.stringify(babelConfig, null, 2)}`);
return babelConfig;
Expand Down
2 changes: 0 additions & 2 deletions packages/stryker-babel-transpiler/src/BabelTranspiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as path from 'path';
import BabelConfigReader from './BabelConfigReader';
import { CONFIG_KEY_FILE } from './helpers/keys';
import { toJSFileName } from './helpers/helpers';
import { setGlobalLogLevel } from 'log4js';

const KNOWN_EXTENSIONS = Object.freeze([
'.es6',
Expand All @@ -21,7 +20,6 @@ class BabelTranspiler implements Transpiler {
private projectRoot: string;

public constructor(options: TranspilerOptions) {
setGlobalLogLevel(options.config.logLevel);
this.babelOptions = new BabelConfigReader().readConfig(options.config);
this.projectRoot = this.determineProjectRoot(options);
if (options.produceSourceMaps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import BabelConfigReader from '../../src/BabelConfigReader';
import { Config } from 'stryker-api/config';
import { expect } from 'chai';
import * as fs from 'fs';
import * as log4js from 'log4js';
import * as log4js from 'stryker-api/logging';
import * as sinon from 'sinon';
import * as path from 'path';

Expand Down
1 change: 0 additions & 1 deletion packages/stryker-html-reporter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"dependencies": {
"file-url": "^2.0.0",
"lodash": "^4.13.1",
"log4js": "^1.0.1",
"mkdirp": "^0.5.1",
"mz": "^2.5.0",
"rimraf": "^2.6.1",
Expand Down
5 changes: 2 additions & 3 deletions packages/stryker-html-reporter/src/HtmlReporter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as log4js from 'log4js';
import * as logging from 'stryker-api/logging';
Copy link
Member

Choose a reason for hiding this comment

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

what else do we use from the logging here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, changing it to a named import.

import fileUrl = require('file-url');
import * as path from 'path';
import { Config } from 'stryker-api/config';
Expand All @@ -7,7 +7,7 @@ import * as util from './util';
import * as templates from './templates';
import Breadcrumb from './Breadcrumb';

const log = log4js.getLogger('HtmlReporter');
const log = logging.getLogger('HtmlReporter');
const DEFAULT_BASE_FOLDER = path.normalize('reports/mutation/html');
export const RESOURCES_DIR_NAME = 'strykerResources';

Expand All @@ -20,7 +20,6 @@ export default class HtmlReporter implements Reporter {
private scoreResult: ScoreResult;

constructor(private options: Config) {
log4js.setGlobalLogLevel(options.logLevel || 'info');
}

onAllSourceFilesRead(files: SourceFile[]) {
Expand Down
2 changes: 1 addition & 1 deletion packages/stryker-html-reporter/test/helpers/log4jsMock.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as log4js from 'log4js';
import * as log4js from 'stryker-api/logging';
Copy link
Member

Choose a reason for hiding this comment

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

This is a log4jsMock file

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I'll change it.

import * as sinon from 'sinon';

let logger = {
Expand Down
3 changes: 2 additions & 1 deletion packages/stryker-jasmine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@
"devDependencies": {
"stryker-api": "^0.17.3"
},
"contributors": []
"contributors": [],
"dependencies": {}
}
1 change: 0 additions & 1 deletion packages/stryker-javascript-mutator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
"babel-generator": "^6.26.0",
"babylon": "^6.18.0",
"lodash": "^4.17.4",
"log4js": "^1.1.1",
"tslib": "^1.8.0"
},
"devDependencies": {
Expand Down
Loading