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

JUnit #103

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

JUnit #103

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
19 changes: 14 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ markdownlint --help
-V, --version output the version number
-f, --fix fix basic errors (does not work with STDIN)
-s, --stdin read from STDIN (does not work with files)
-o, --output [outputFile] write issues to file (no console)
-c, --config [configFile] configuration file (JSON, JSONC, JS, or YAML)
-i, --ignore [file|directory|glob] file(s) to ignore/exclude
-p, --ignore-path [file] path to file with ignore pattern(s)
-r, --rules [file|directory|glob|package] custom rule files
-o, --output <outputFile> write issues to file (no console)
-j, --junit <junitFile> write issues to file in JUnit format and to the console
-c, --config <configFile> configuration file (JSON, JSONC, JS, or YAML)
-i, --ignore <file|directory|glob> file(s) to ignore/exclude
-p, --ignore-path <file> path to file with ignore pattern(s)
-r, --rules <file|directory|glob|package> custom rule files
```

### Globbing
Expand Down Expand Up @@ -63,6 +64,13 @@ Because this option makes changes to the input files, it is good to make a backu

> Because not all rules include fix information when reporting errors, fixes may overlap, and not all errors are fixable, `--fix` will not usually address all errors.

### JUnit report

If the `-j`/`--junit` option is specified, `markdownlint-cli` will output an XML report in JUnit format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add link to some info about “JUnit format”, maybe?

This report can be used in CI/CD environments to make the `markdownlint-cli` results available to your CI/CD tool.
If both `-j`/`--junit` and `-o`/`--output` are specified, `-o`/`--output` will take precedence.
Only one of these options should be specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe remove this, I think your previous line is clear.


## Configuration

`markdownlint-cli` reuses [the rules][rules] from `markdownlint` package.
Expand Down Expand Up @@ -100,6 +108,7 @@ A JS configuration file may internally `require` one or more npm packages as a w
- `1`: Linting errors / bad parameter
- `2`: Unable to write `-o`/`--output` output file
- `3`: Unable to load `-r`/`--rules` custom rule
- `4`: Unable to write `-j`/`--junit` output file

## Related

Expand Down
51 changes: 43 additions & 8 deletions markdownlint.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function readConfiguration(args) {
markdownlint.readConfigSync(userConfigFile, configFileParsers);
config = require('deep-extend')(config, userConfig);
} catch (error) {
console.warn('Cannot read or parse config file ' + userConfigFile + ': ' + error.message);
console.warn(`Cannot read or parse config file ${userConfigFile}: ${error.message}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for doing the others!

}
}

Expand Down Expand Up @@ -165,9 +165,43 @@ function printResult(lintResult) {
try {
fs.writeFileSync(program.output, lintResultString);
} catch (error) {
console.warn('Cannot write to output file ' + program.output + ': ' + error.message);
console.warn(`Cannot write to output file ${program.output}: ${error.message}`);
process.exitCode = 2;
}
} else if (program.junit) {
const builder = require('junit-report-builder');
const testSuite = builder
.testSuite()
.name('markdownlint')
.timestamp(new Date().toISOString())
.time(0);
if (results.length > 0) {
results.forEach(result => {
const {file, lineNumber, column, names, description} = result;
const columnText = column ? `:${column}` : '';
const testName = `${file}:${lineNumber}${columnText} ${names}`;
testSuite
.testCase()
.className(file)
.name(testName)
.failure(`${testName} ${description}`, names)
.time(0);
});
} else {
const className = program.stdin ? 'stdin' : program.args;
testSuite.testCase().className(className).name('markdownlint').time(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make this multi-line for consistency with above?

}

try {
builder.writeTo(program.junit);
} catch (error) {
console.warn(`Cannot write to JUnit file ${program.junit}: ${error.message}`);
process.exitCode = 4;
}

if (lintResultString) {
console.error(lintResultString);
}
} else if (lintResultString) {
console.error(lintResultString);
}
Expand All @@ -184,11 +218,12 @@ program
.usage('[options] <files|directories|globs>')
FISHMANPET marked this conversation as resolved.
Show resolved Hide resolved
FISHMANPET marked this conversation as resolved.
Show resolved Hide resolved
.option('-f, --fix', 'fix basic errors (does not work with STDIN)')
.option('-s, --stdin', 'read from STDIN (does not work with files)')
.option('-o, --output [outputFile]', 'write issues to file (no console)')
.option('-c, --config [configFile]', 'configuration file (JSON, JSONC, JS, or YAML)')
.option('-i, --ignore [file|directory|glob]', 'file(s) to ignore/exclude', concatArray, [])
.option('-p, --ignore-path [file]', 'path to file with ignore pattern(s)')
.option('-r, --rules [file|directory|glob|package]', 'custom rule files', concatArray, []);
.option('-o, --output <outputFile>', 'write issues to file (no console)')
FISHMANPET marked this conversation as resolved.
Show resolved Hide resolved
.option('-j, --junit <junitFile>', 'write issues to file in JUnit format and to the console')
.option('-c, --config <configFile>', 'configuration file (JSON, JSONC, JS, or YAML)')
.option('-i, --ignore <file|directory|glob>', 'file(s) to ignore/exclude', concatArray, [])
.option('-p, --ignore-path <file>', 'path to file with ignore pattern(s)')
.option('-r, --rules <file|directory|glob|package>', 'custom rule files', concatArray, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just noticed the extra space here, mind removing it?


program.parse(process.argv);

Expand Down Expand Up @@ -227,7 +262,7 @@ function loadCustomRules(rules) {

return fileList;
} catch (error) {
console.error('Cannot load custom rule ' + rule + ': ' + error.message);
console.error(`Cannot load custom rule ${rule}: ${error.message}`);
process.exit(3);
}
}));
Expand Down
54 changes: 52 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"ignore": "~5.1.4",
"js-yaml": "~3.13.1",
"jsonc-parser": "~2.2.0",
"junit-report-builder": "~2.0.0",
"lodash.differencewith": "~4.5.0",
"lodash.flatten": "~4.4.0",
"markdownlint": "~0.20.3",
Expand All @@ -58,6 +59,7 @@
"husky": "^3.0.4",
"tap-growl": "^3.0.0",
"test-rule-package": "./test/custom-rules/test-rule-package",
"xml-js": "^1.6.11",
"xo": "*"
},
"xo": {
Expand Down
80 changes: 80 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const fs = require('fs');
const path = require('path');
const test = require('ava');
const execa = require('execa');
const xmlParser = require('xml-js');

const errorPattern = /(\.md|\.markdown|\.mdf|stdin):\d+(:\d+)? MD\d{3}/gm;

Expand Down Expand Up @@ -357,6 +358,85 @@ test('--output with invalid path fails', async t => {
}
});

test('--junit with empty input has single successful test', async t => {
const input = '';
const junit = '../junitA.xml';
const result = await execa('../markdownlint.js',
['--stdin', '--junit', junit],
{input, stripFinalNewline: false});
const xml = fs.readFileSync(junit, 'utf8');
const parsedXml = xmlParser.xml2js(xml, {compact: true});
t.is(result.stdout, '');
t.is(result.stderr, '');
t.is(Object.keys(parsedXml.testsuites).length, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my separate comment about maybe just comparing the output text directly. What you have here is more correct and thorough, but it’s a lot more typing and harder to follow. I’m fine with either choice - up to you if you want to type more. :)

t.is(Object.keys(parsedXml.testsuites.testsuite).length, 2);
t.is(parsedXml.testsuites.testsuite._attributes.name, 'markdownlint');
t.is(parsedXml.testsuites.testsuite._attributes.timestamp.match(/\d{4}-[01]\d-[0-3]\dT[0-2](?:\d:[0-5]){2}\d\.\d+Z/gm).length, 1);
t.is(parsedXml.testsuites.testsuite._attributes.time, '0');
t.is(parsedXml.testsuites.testsuite._attributes.tests, '1');
t.is(parsedXml.testsuites.testsuite._attributes.failures, '0');
t.is(parsedXml.testsuites.testsuite._attributes.errors, '0');
t.is(parsedXml.testsuites.testsuite._attributes.skipped, '0');
t.is(parsedXml.testsuites.testsuite.testcase._attributes.classname, 'stdin');
t.is(parsedXml.testsuites.testsuite.testcase._attributes.name, 'markdownlint');
t.is(parsedXml.testsuites.testsuite.testcase._attributes.time, '0');
fs.unlinkSync(junit);
});

// WIP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your test cases make sense to me!

// test('--junit with valid input has empty output', async t => {
// const input = [
// '# Heading',
// '',
// 'Text',
// ''
// ].join('\n');
// const output = '../outputB.txt';
// const result = await execa('../markdownlint.js',
// ['--stdin', '--output', output],
// {input, stripFinalNewline: false});
// t.is(result.stdout, '');
// t.is(result.stderr, '');
// t.is(fs.readFileSync(output, 'utf8'), '');
// fs.unlinkSync(output);
// });

// test('--junit with invalid input outputs violations', async t => {
// const input = [
// 'Heading',
// '',
// 'Text ',
// ''
// ].join('\n');
// const output = '../outputC.txt';
// try {
// await execa('../markdownlint.js',
// ['--stdin', '--output', output],
// {input, stripFinalNewline: false});
// t.fail();
// } catch (error) {
// t.is(error.stdout, '');
// t.is(error.stderr, '');
// t.is(fs.readFileSync(output, 'utf8').match(errorPattern).length, 2);
// fs.unlinkSync(output);
// }
// });

// test('--junit with invalid path fails', async t => {
// const input = '';
// const output = 'invalid/outputD.txt';
// try {
// await execa('../markdownlint.js',
// ['--stdin', '--output', output],
// {input, stripFinalNewline: false});
// t.fail();
// } catch (error) {
// t.is(error.stdout, '');
// t.is(error.stderr.replace(/: ENOENT[^]*$/, ''), 'Cannot write to output file ' + output);
// t.throws(() => fs.accessSync(output, 'utf8'));
// }
// });

test('configuration file can be YAML', async t => {
const result = await execa('../markdownlint.js',
['--config', 'md043-config.yaml', 'md043-config.md'],
Expand Down