From 6ed4c54ba6599170900f65161bd616c573a81da7 Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Mon, 6 May 2019 17:17:44 +0200 Subject: [PATCH 1/3] refactor: ditch 'handleRuntimeProblems' and 'blueprintUtils' Close #1191, close #916, close #127 --- lib/Dredd.js | 24 +- lib/annotationToLoggerInfo.js | 96 ++++++++ lib/blueprintUtils.js | 76 ------ lib/compileTransactionName.js | 14 ++ lib/handleRuntimeProblems.js | 43 ---- test/integration/annotations-test.js | 36 ++- .../integration/cli/api-blueprint-cli-test.js | 4 +- test/integration/cli/openapi2-cli-test.js | 4 +- test/unit/annotationToLoggerInfo-test.js | 145 +++++++++++ test/unit/blueprintUtils-test.js | 233 ------------------ test/unit/handleRuntimeProblems-test.js | 125 ---------- 11 files changed, 302 insertions(+), 498 deletions(-) create mode 100644 lib/annotationToLoggerInfo.js delete mode 100644 lib/blueprintUtils.js create mode 100644 lib/compileTransactionName.js delete mode 100644 lib/handleRuntimeProblems.js create mode 100644 test/unit/annotationToLoggerInfo-test.js delete mode 100644 test/unit/blueprintUtils-test.js delete mode 100644 test/unit/handleRuntimeProblems-test.js diff --git a/lib/Dredd.js b/lib/Dredd.js index a505a3c03..ff3968011 100644 --- a/lib/Dredd.js +++ b/lib/Dredd.js @@ -3,13 +3,13 @@ const parse = require('dredd-transactions/parse'); const compile = require('dredd-transactions/compile'); const configureReporters = require('./configureReporters'); -const handleRuntimeProblems = require('./handleRuntimeProblems'); const resolveLocations = require('./resolveLocations'); const readLocation = require('./readLocation'); const resolveModule = require('./resolveModule'); const logger = require('./logger'); const TransactionRunner = require('./TransactionRunner'); const { applyConfiguration } = require('./configuration'); +const annotationToLoggerInfo = require('./annotationToLoggerInfo'); function prefixError(error, prefix) { @@ -89,6 +89,14 @@ function toTransactions(apiDescriptions) { } +function toLoggerInfos(apiDescriptions) { + return apiDescriptions + .map(apiDescription => apiDescription.annotations + .map(annotation => annotationToLoggerInfo(apiDescription.location, annotation))) + .reduce((flatAnnotations, annotations) => flatAnnotations.concat(annotations), []); +} + + class Dredd { constructor(config) { this.configuration = applyConfiguration(config); @@ -165,13 +173,19 @@ class Dredd { this.logger.debug('Preparing API description documents'); this.prepareAPIdescriptions((error, apiDescriptions) => { if (error) { callback(error, this.stats); return; } - this.configuration.apiDescriptions = apiDescriptions; - // TODO https://github.com/apiaryio/dredd/issues/1191 - const annotationsError = handleRuntimeProblems(apiDescriptions, this.logger); - if (annotationsError) { callback(annotationsError, this.stats); return; } + const loggerInfos = toLoggerInfos(apiDescriptions); + // FIXME: Winston 3.x supports calling .log() directly with the loggerInfo + // object as it's sole argument, but that's not the case with Winston 2.x + // Once we upgrade Winston, the line below can be simplified to .log(loggerInfo) + loggerInfos.forEach(({ level, message }) => this.logger.log(level, message)); + if (loggerInfos.find(loggerInfo => loggerInfo.level === 'error')) { + callback(new Error('API description processing error'), this.stats); + return; + } this.logger.debug('Starting the transaction runner'); + this.configuration.apiDescriptions = apiDescriptions; this.transactionRunner.config(this.configuration); const transactions = toTransactions(apiDescriptions); this.transactionRunner.run(transactions, (runError) => { diff --git a/lib/annotationToLoggerInfo.js b/lib/annotationToLoggerInfo.js new file mode 100644 index 000000000..048ecf02f --- /dev/null +++ b/lib/annotationToLoggerInfo.js @@ -0,0 +1,96 @@ +const compileTransactionName = require('./compileTransactionName'); + + +/** + * Turns annotation type into a log level + */ +function typeToLogLevel(annotationType) { + const level = { error: 'error', warning: 'warn' }[annotationType]; + if (!level) { + throw new Error(`Invalid annotation type: '${annotationType}'`); + } + return level; +} + + +/** + * Takes a component identifier and turns it into something user can understand + * + * @param {string} component + */ +function formatComponent(component) { + switch (component) { + case 'apiDescriptionParser': + return 'API description parser'; + case 'parametersValidation': + return 'API description URI parameters validation'; + case 'uriTemplateExpansion': + return 'API description URI template expansion'; + default: + return 'API description'; + } +} + + +/** + * Formats given location data as something user can understand + * + * @param {string} apiDescriptionLocation API description location name + * @param {array} annotationLocation See 'dredd-transactions' docs + */ +function formatLocation(apiDescriptionLocation, annotationLocation) { + if (!annotationLocation) { + return apiDescriptionLocation; + } + + const [[startLine, startColumn], [endLine, endColumn]] = annotationLocation; + const editorLink = `${apiDescriptionLocation}:${startLine}`; + const from = `line ${startLine} column ${startColumn}`; + + if (startLine === endLine && startColumn === endColumn) { + return `${editorLink} (${from})`; + } + + const to = startLine === endLine + ? `column ${endColumn}` + : `line ${endLine} column ${endColumn}`; + return `${editorLink} (from ${from} to ${to})`; +} + + +/** + * @typedef {Object} LoggerInfo A plain object winston.log() accepts as input + * @property {string} level + * @property {string} message + */ + +/** + * Takes API description parser or compiler annotation returned from + * the 'dredd-transactions' library and transforms it into a message + * Dredd can show to the user. Returns an object logger accepts as input. + * + * @param {string} apiDescriptionLocation API description location name + * @param {Object} annotation the annotation object from Dredd Transactions + * @return {LoggerInfo} + */ +module.exports = function annotationToLoggerInfo(apiDescriptionLocation, annotation) { + const level = typeToLogLevel(annotation.type); + + if (annotation.component === 'apiDescriptionParser') { + const message = ( + `${formatComponent(annotation.component)} ${annotation.type}` + + ` in ${formatLocation(apiDescriptionLocation, annotation.location)}:` + + ` ${annotation.message}` + ); + return { level, message }; + } + + // See https://github.com/apiaryio/dredd-transactions/issues/275 why this + // is handled in a different way than parser annotations + const message = ( + `${formatComponent(annotation.component)} ${annotation.type}` + + ` in ${apiDescriptionLocation} (${compileTransactionName(annotation.origin)}):` + + ` ${annotation.message}` + ); + return { level, message }; +}; diff --git a/lib/blueprintUtils.js b/lib/blueprintUtils.js deleted file mode 100644 index d18b5f5c4..000000000 --- a/lib/blueprintUtils.js +++ /dev/null @@ -1,76 +0,0 @@ -const NEWLINE_RE = /\n/g; - -function characterIndexToPosition(charIndex = 0, code = '') { - const codeFragment = code.substring(0, charIndex); - const match = codeFragment.match(NEWLINE_RE) || []; - const row = match.length + 1; - return { row }; -} - -const sortNumbersAscending = (a, b) => a - b; - -function warningLocationToRanges(warningLocation = [], text = '') { - if (!warningLocation.length) { - // No start-end ranges, nothing to return - return []; - } - - const rowsIndexes = []; - - let position = characterIndexToPosition(warningLocation[0][0], text); - - // Add this warning position row into ranges array - rowsIndexes.push(position.row); - - if (warningLocation.length > 0) { - // More lines - for (let locKey = 0; locKey < warningLocation.length; locKey++) { - const loc = warningLocation[locKey]; - if (locKey > 0) { - position = characterIndexToPosition(loc[0], text); - rowsIndexes.push(position.row); - } - } - } - - rowsIndexes.sort(sortNumbersAscending); - - const ranges = []; - let range = { start: rowsIndexes[0], end: rowsIndexes[0] }; - for (const rowIndex of rowsIndexes) { - if ((rowIndex === range.end) || (rowIndex === (range.end + 1))) { // Moving end of known range - range.end = rowIndex; - } else { - ranges.push(range); // non-continuous range - range = { start: rowIndex, end: rowIndex }; - } - } - // Push the last edited range to ranges-array - ranges.push(range); - - return ranges; -} - -function rangesToLinesText(ranges) { - let pos = ''; - const iterable = ranges || []; - for (let rangeIndex = 0; rangeIndex < iterable.length; rangeIndex++) { - const range = iterable[rangeIndex]; - if (rangeIndex > 0) { - pos += ', '; - } - if (range.start !== range.end) { - pos += `lines ${range.start}-${range.end}`; - } else { - pos += `line ${range.start}`; - } - } - return pos; -} - -module.exports = { - characterIndexToPosition, - rangesToLinesText, - sortNumbersAscending, - warningLocationToRanges, -}; diff --git a/lib/compileTransactionName.js b/lib/compileTransactionName.js new file mode 100644 index 000000000..591be727c --- /dev/null +++ b/lib/compileTransactionName.js @@ -0,0 +1,14 @@ +// This file is copy-pasted "as is" from the Dredd Transactions library, where +// it's also tested. This is a temporary solution, +// see https://github.com/apiaryio/dredd-transactions/issues/276 + + +module.exports = function compileTransactionName(origin) { + const segments = []; + if (origin.apiName) { segments.push(origin.apiName); } + if (origin.resourceGroupName) { segments.push(origin.resourceGroupName); } + if (origin.resourceName) { segments.push(origin.resourceName); } + if (origin.actionName) { segments.push(origin.actionName); } + if (origin.exampleName) { segments.push(origin.exampleName); } + return segments.join(' > '); +}; diff --git a/lib/handleRuntimeProblems.js b/lib/handleRuntimeProblems.js deleted file mode 100644 index 6debe9776..000000000 --- a/lib/handleRuntimeProblems.js +++ /dev/null @@ -1,43 +0,0 @@ -const blueprintUtils = require('./blueprintUtils'); -const defaultLogger = require('./logger'); - - -module.exports = function handleRuntimeProblems(apiDescriptions, logger) { - logger = logger || defaultLogger; - - let error = false; - - apiDescriptions.forEach((apiDescription) => { - for (const annotation of apiDescription.annotations) { - let log; - let message; - if (annotation.type === 'warning') { - log = logger.warn; - } else { - error = true; - log = logger.error; - } - - if (annotation.component === 'apiDescriptionParser') { - const ranges = blueprintUtils.warningLocationToRanges(annotation.location, apiDescription.content); - message = `Parser ${annotation.type} in '${apiDescription.location}': ${annotation.message}`; - if (ranges && ranges.length) { - message += ` on ${blueprintUtils.rangesToLinesText(ranges)}`; - } - log(message); - } else { - const transactionName = [ - annotation.origin.apiName, - annotation.origin.resourceGroupName, - annotation.origin.resourceName, - annotation.origin.actionName, - ].filter(part => !!part).join(' > '); - log(`Compilation ${annotation.type} in '${apiDescription.location}': ${annotation.message} (${transactionName})`); - } - } - }); - - if (error) { - return new Error('Error when processing API description.'); - } -}; diff --git a/test/integration/annotations-test.js b/test/integration/annotations-test.js index 8899ebe73..9f5d7fbfb 100644 --- a/test/integration/annotations-test.js +++ b/test/integration/annotations-test.js @@ -14,7 +14,7 @@ function compileTransactions(apiDescription, logger, callback) { describe('Parser and compiler annotations', () => { describe('when processing a file with parser warnings', () => { - const logger = { debug: sinon.spy(), warn: sinon.spy() }; + const logger = { debug: sinon.spy(), log: sinon.spy() }; let error; before((done) => { @@ -32,16 +32,19 @@ FORMAT: 1A it("doesn't abort Dredd", () => { assert.isUndefined(error); }); + it('logs warnings', () => { + assert.equal(logger.log.getCall(0).args[0], 'warn'); + }); it('logs the warnings with line numbers', () => { assert.match( - logger.warn.getCall(0).args[0], - /^parser warning in 'configuration\.apiDescriptions\[0\]': [\s\S]+ on line 5$/i + logger.log.getCall(0).args[1], + /parser warning in configuration\.apiDescriptions\[0\]:5 \(from line 5 column 3 to column 11\)/i ); }); }); describe('when processing a file with parser errors', () => { - const logger = { debug: sinon.spy(), error: sinon.spy() }; + const logger = { debug: sinon.spy(), log: sinon.spy() }; let error; before((done) => { @@ -60,16 +63,19 @@ FORMAT: 1A it('aborts Dredd', () => { assert.instanceOf(error, Error); }); + it('logs errors', () => { + assert.equal(logger.log.getCall(0).args[0], 'error'); + }); it('logs the errors with line numbers', () => { assert.match( - logger.error.getCall(0).args[0], - /^parser error in 'configuration\.apiDescriptions\[0\]': [\s\S]+ on line 6$/i + logger.log.getCall(0).args[1], + /parser error in configuration\.apiDescriptions\[0\]:6 \(line 6 column 1\)/i ); }); }); describe('when processing a file with compilation warnings', () => { - const logger = { debug: sinon.spy(), warn: sinon.spy() }; + const logger = { debug: sinon.spy(), log: sinon.spy() }; let error; before((done) => { @@ -87,16 +93,19 @@ FORMAT: 1A it("doesn't abort Dredd", () => { assert.isUndefined(error); }); + it('logs warnings', () => { + assert.equal(logger.log.getCall(0).args[0], 'warn'); + }); it('logs the warnings with a transaction path', () => { assert.match( - logger.warn.getCall(0).args[0], - /^compilation warning in 'configuration\.apiDescriptions\[0\]': [\s\S]+ \(Dummy API > Index > Index\)$/i + logger.log.getCall(0).args[1], + /uri template expansion warning in configuration\.apiDescriptions\[0\] \(Dummy API > Index > Index\)/i ); }); }); describe('when processing a file with compilation errors', () => { - const logger = { debug: sinon.spy(), error: sinon.spy(), warn: sinon.spy() }; + const logger = { debug: sinon.spy(), log: sinon.spy() }; let error; before((done) => { @@ -116,10 +125,13 @@ FORMAT: 1A it('aborts Dredd', () => { assert.instanceOf(error, Error); }); + it('logs errors', () => { + assert.equal(logger.log.getCall(0).args[0], 'error'); + }); it('logs the errors with a transaction path', () => { assert.match( - logger.error.getCall(0).args[0], - /^compilation error in 'configuration\.apiDescriptions\[0\]': [\s\S]+ \(Dummy API > Index > Index\)$/i + logger.log.getCall(0).args[1], + /uri parameters validation error in configuration\.apiDescriptions\[0\] \(Dummy API > Index > Index\)/i ); }); }); diff --git a/test/integration/cli/api-blueprint-cli-test.js b/test/integration/cli/api-blueprint-cli-test.js index 9ae718548..43d592199 100644 --- a/test/integration/cli/api-blueprint-cli-test.js +++ b/test/integration/cli/api-blueprint-cli-test.js @@ -38,7 +38,7 @@ describe('CLI - API Blueprint Document', () => { }); it('should exit with status 1', () => assert.equal(runtimeInfo.dredd.exitStatus, 1)); - it('should print error message to stderr', () => assert.include(runtimeInfo.dredd.stderr, 'Error when processing API description')); + it('should print error message to stderr', () => assert.include(runtimeInfo.dredd.stderr, 'API description processing error')); }); describe('when API Blueprint is loaded with warnings', () => { @@ -58,7 +58,7 @@ describe('CLI - API Blueprint Document', () => { }); it('should exit with status 0', () => assert.equal(runtimeInfo.dredd.exitStatus, 0)); - it('should print warning to stdout', () => assert.include(runtimeInfo.dredd.stdout, 'warn: Compilation warning')); + it('should print warning to stdout', () => assert.include(runtimeInfo.dredd.stdout, 'API description URI template expansion warning')); }); }); }); diff --git a/test/integration/cli/openapi2-cli-test.js b/test/integration/cli/openapi2-cli-test.js index 42ccddd2f..6cc8347e6 100644 --- a/test/integration/cli/openapi2-cli-test.js +++ b/test/integration/cli/openapi2-cli-test.js @@ -38,7 +38,7 @@ describe('CLI - OpenAPI 2 Document', () => { }); it('should exit with status 1', () => assert.equal(runtimeInfo.dredd.exitStatus, 1)); - it('should print error message to stderr', () => assert.include(runtimeInfo.dredd.stderr, 'Error when processing API description')); + it('should print error message to stderr', () => assert.include(runtimeInfo.dredd.stderr, 'API description processing error')); }); describe('when OpenAPI 2 is loaded with warnings', () => { @@ -60,7 +60,7 @@ describe('CLI - OpenAPI 2 Document', () => { }); it('should exit with status 0', () => assert.equal(runtimeInfo.dredd.exitStatus, 0)); - it('should print warning to stdout', () => assert.include(runtimeInfo.dredd.stdout, 'warn: Parser warning')); + it('should print warning to stdout', () => assert.include(runtimeInfo.dredd.stdout, 'API description parser warning')); }); }); }); diff --git a/test/unit/annotationToLoggerInfo-test.js b/test/unit/annotationToLoggerInfo-test.js new file mode 100644 index 000000000..4ed21a85a --- /dev/null +++ b/test/unit/annotationToLoggerInfo-test.js @@ -0,0 +1,145 @@ +const { assert } = require('chai'); + +const annotationToLoggerInfo = require('../../lib/annotationToLoggerInfo'); + + +const PARSE_ANNOTATION_FIXTURE = { + type: 'error', + message: 'Ouch!', + component: 'apiDescriptionParser', + location: [[1, 2], [3, 4]], +}; + +const COMPILE_ANNOTATION_FIXTURE = { + type: 'error', + message: 'Ouch!', + component: 'uriTemplateExpansion', + origin: { apiName: 'Broken API', resourceName: 'Things', actionName: 'Retrieve Things' }, +}; + + +describe('annotationToLoggerInfo()', () => { + describe('annotation.type', () => { + it('chooses error logging level for error annotation type', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + type: 'error', + }); + assert.equal(loggerInfo.level, 'error'); + }); + it('chooses warn logging level for warning annotation type', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + type: 'warning', + }); + assert.equal(loggerInfo.level, 'warn'); + }); + it('throws for invalid annotation type', () => { + assert.throws(() => annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + type: 'gargamel', + }), 'gargamel'); + }); + it('propagates the type to the message for parse annotation', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + type: 'warning', + }); + assert.match(loggerInfo.message, /^API description [\s\S]+ warning in/); + }); + it('propagates the type to the message for compile annotation', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...COMPILE_ANNOTATION_FIXTURE, + type: 'warning', + }); + assert.match(loggerInfo.message, /^API description [\s\S]+ warning in/); + }); + }); + + describe('annotation.component', () => { + it('formats apiDescriptionParser', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + component: 'apiDescriptionParser', + }); + assert.match(loggerInfo.message, /^API description parser error/); + }); + it('formats parametersValidation', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...COMPILE_ANNOTATION_FIXTURE, + component: 'parametersValidation', + }); + assert.match(loggerInfo.message, /^API description URI parameters validation error/); + }); + it('formats uriTemplateExpansion', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...COMPILE_ANNOTATION_FIXTURE, + component: 'uriTemplateExpansion', + }); + assert.match(loggerInfo.message, /^API description URI template expansion error/); + }); + it('formats unexpected component with a generic name', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...COMPILE_ANNOTATION_FIXTURE, + component: 'gargamel', + }); + assert.match(loggerInfo.message, /^API description error/); + }); + }); + + describe('annotation.origin', () => { + it('uses transaction name as a location hint for compile annotations', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...COMPILE_ANNOTATION_FIXTURE, + component: 'parametersValidation', + }); + assert.include( + loggerInfo.message, + 'error in apiary.apib (Broken API > Things > Retrieve Things): Ouch!' + ); + }); + }); + + describe('annotation.location', () => { + it('formats location for parse annotations', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + location: [[1, 2], [3, 4]], + }); + assert.include( + loggerInfo.message, + 'error in apiary.apib:1 (from line 1 column 2 to line 3 column 4): Ouch!' + ); + }); + it('formats location without end line if it is the same as the start line', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + location: [[1, 2], [1, 4]], + }); + assert.include( + loggerInfo.message, + 'error in apiary.apib:1 (from line 1 column 2 to column 4): Ouch!' + ); + }); + it('formats location without range if the start and the end are the same', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + location: [[1, 2], [1, 2]], + }); + assert.include( + loggerInfo.message, + 'error in apiary.apib:1 (line 1 column 2): Ouch!' + ); + }); + it('formats missing location', () => { + const loggerInfo = annotationToLoggerInfo('apiary.apib', { + ...PARSE_ANNOTATION_FIXTURE, + location: null, + }); + assert.include( + loggerInfo.message, + 'error in apiary.apib: Ouch!' + ); + }); + }); +}); diff --git a/test/unit/blueprintUtils-test.js b/test/unit/blueprintUtils-test.js deleted file mode 100644 index 053bac7ca..000000000 --- a/test/unit/blueprintUtils-test.js +++ /dev/null @@ -1,233 +0,0 @@ -const { assert } = require('chai'); - -const blueprintUtils = require('../../lib/blueprintUtils'); - -describe('blueprintUtils', () => { - const placeholderText = ''; - - describe('characterIndexToPosition()', () => { - describe('under standard circumstances', () => it('returns an object with non-zero-based row', () => { - const str = 'first\nsecond\nthird lines\ncontent continues'; - const position = blueprintUtils.characterIndexToPosition(str.indexOf('lines', str), str); - assert.deepEqual(position, { row: 3 }); - })); - - describe('when given one-line input and zero index', () => it('returns an object with row 1', () => { - const str = 'hello\n'; - const position = blueprintUtils.characterIndexToPosition(str.indexOf('hello', str), str); - assert.deepEqual(position, { row: 1 }); - })); - }); - - describe('warningLocationToRanges()', () => { - let str = null; - let location = []; - - it('keeps ranges that follow each other line-numbers, but also resolves single-lines', () => { - str = 'one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\nten'; - location = [ - [str.indexOf('two'), 2], - [str.indexOf('three'), 2], - [str.indexOf('four'), 2], - // Keep some lines of - [str.indexOf('six'), 2], - [str.indexOf('seven'), 2], - [str.indexOf('eight'), 2], - // Also add just one single line warning location - [str.indexOf('ten'), 3], - ]; - const ranges = blueprintUtils.warningLocationToRanges(location, str); - assert.isArray(ranges); - assert.lengthOf(ranges, 3); - assert.deepEqual(ranges, [ - { start: 2, end: 4 }, - { start: 6, end: 8 }, - { start: 10, end: 10 }, - ]); - }); - - it('works for some API description warnings too', () => { - const apiDescription = `\ -# Indented API - -## GET /url -+ Response 200 (text/plain) - - wrongly indented - resp.body - -+ Response 404 (text/plain) - - ok indentation\ -`; - const annotation = { - element: 'annotation', - meta: { classes: ['warning'] }, - attributes: { - code: 10, - sourceMap: [ - { - element: 'sourceMap', - content: [[59, 17], [78, 10]], - }, - ], - }, - content: - 'message-body asset is expected to be a pre-formatted code ' - + 'block, every of its line indented by exactly 8 spaces or 2 tabs', - }; - - location = []; - for (const sourceMap of annotation.attributes.sourceMap) { - location = location.concat(sourceMap.content); - } - assert.isAbove(location.length, 0); - - const ranges = blueprintUtils.warningLocationToRanges(location, apiDescription); - assert.isArray(ranges); - assert.lengthOf(ranges, 1); - assert.deepEqual(ranges, [{ start: 6, end: 7 }]); - }); - - it('returns an empty Array for empty locations', () => assert.deepEqual(blueprintUtils.warningLocationToRanges([], placeholderText), [])); - - it('returns an empty Array for undefined locations', () => assert.deepEqual(blueprintUtils.warningLocationToRanges(undefined, placeholderText), [])); - }); - - describe('rangesToLinesText()', () => { - describe('when tested on fake locations', () => it('should return a string of line(s) separated with comma', () => { - const line = blueprintUtils.rangesToLinesText([ - { start: 2, end: 4 }, - { start: 8, end: 8 }, - { start: 10, end: 15 }, - ]); - assert.strictEqual(line, 'lines 2-4, line 8, lines 10-15'); - })); - - describe('for a real API description document', () => { - const allRanges = []; - const apiDescription = `\ -# Indentation warnings API -## GET /url -+ Response 200 (text/plain) - - badly indented 5. line - responsing body 6. line - -+ Response 400 (text/plain) - - + Headers - - headers-should-be:preformatted_12_line - - + Body - - not-enough indentation 16th line - -## POST /create -+ Request (text/plain) - - is it body? - maybe it is - if you say so - -+ Response 201 - - yup!\ -`; - const annotations = [ - { - element: 'annotation', - meta: { classes: ['warning'] }, - attributes: { - code: 10, - sourceMap: [ - { - element: 'sourceMap', - content: [[70, 23], [95, 24]], - }, - ], - }, - content: - 'message-body asset is expected to be a pre-formatted code ' - + 'block, every of its line indented by exactly 8 spaces or 2 tabs', - }, - { - element: 'annotation', - meta: { classes: ['warning'] }, - attributes: { - code: 10, - sourceMap: [ - { - element: 'sourceMap', - content: [[168, 39]], - }, - ], - }, - content: - 'headers is expected to be a pre-formatted code block, every ' - + 'of its line indented by exactly 12 spaces or 3 tabs', - }, - { - element: 'annotation', - meta: { classes: ['warning'] }, - attributes: { - code: 10, - sourceMap: [ - { - element: 'sourceMap', - content: [[224, 33]], - }, - ], - }, - content: - 'message-body asset is expected to be a pre-formatted code ' - + 'block, every of its line indented by exactly 12 spaces or 3 tabs', - }, - { - element: 'annotation', - meta: { classes: ['warning'] }, - attributes: { - code: 10, - sourceMap: [ - { - element: 'sourceMap', - content: [[302, 12], [318, 12], [334, 14]], - }, - ], - }, - content: - 'message-body asset is expected to be a pre-formatted code ' - + 'block, every of its line indented by exactly 8 spaces or 2 tabs', - }, - ]; - - for (const annotation of annotations) { - let location = []; - for (const sourceMap of annotation.attributes.sourceMap) { - location = location.concat(sourceMap.content); - } - allRanges.push(blueprintUtils.warningLocationToRanges(location, apiDescription)); - } - - it('shows ~ 4 warnings', () => assert.equal(annotations.length, 4)); - - it('prints lines for those warnings', () => { - const expectedLines = [ - 'lines 5-6', - 'line 12', - 'line 16', - 'lines 21-23', - ]; - const result = []; - for (let lineIndex = 0; lineIndex < expectedLines.length; lineIndex++) { - const expectedLine = expectedLines[lineIndex]; - const generatedLine = blueprintUtils.rangesToLinesText(allRanges[lineIndex]); - assert.isString(expectedLine); - result.push(assert.strictEqual(generatedLine, expectedLine)); - } - return result; - }); - }); - }); -}); diff --git a/test/unit/handleRuntimeProblems-test.js b/test/unit/handleRuntimeProblems-test.js deleted file mode 100644 index 883ee8520..000000000 --- a/test/unit/handleRuntimeProblems-test.js +++ /dev/null @@ -1,125 +0,0 @@ -const proxyquire = require('proxyquire'); -const sinon = require('sinon'); -const { assert } = require('chai'); - -const parse = require('dredd-transactions/parse'); -const compile = require('dredd-transactions/compile'); - -const logger = require('../../lib/logger'); - -const handleRuntimeProblems = proxyquire('../../lib/handleRuntimeProblems', - { './logger': logger }); - -function prepareData(apiDescriptionDocument, filename, done) { - parse(apiDescriptionDocument, (err, parseResult) => { - if (err) { done(err); return; } - - const { annotations } = compile(parseResult.mediaType, parseResult.apiElements, filename); - done(null, [ - { content: apiDescriptionDocument, location: filename, annotations }, - ]); - }); -} - -describe('handleRuntimeProblems()', () => { - let warnOutput; - let errorOutput; - - beforeEach(() => { - warnOutput = ''; - errorOutput = ''; - - sinon.stub(logger, 'warn').callsFake((...args) => { warnOutput += args.join(' ').toLowerCase(); }); - sinon.stub(logger, 'error').callsFake((...args) => { errorOutput += args.join(' ').toLowerCase(); }); - }); - - afterEach(() => { - logger.warn.restore(); - logger.error.restore(); - }); - - describe('Prints parser error', () => { - let error; - - const apiDescriptionDocument = `\ -FORMAT: 1A -# Beehive API -\t\t\ -`; - const filename = 'dummy-filename.apib'; - - beforeEach(done => prepareData(apiDescriptionDocument, filename, (err, data) => { - if (err) { return done(err); } - error = handleRuntimeProblems(data); - done(); - })); - - it('returns error', () => assert.isOk(error)); - it('has no warning output', () => assert.equal(warnOutput, '')); - it('has error output', () => assert.isOk(errorOutput)); - context('the error output', () => { - it('mentions it is from parser', () => assert.include(errorOutput, 'parser')); - it('mentions it is error', () => assert.include(errorOutput, 'error')); - it('mentions the filename', () => assert.include(errorOutput, filename)); - it('mentions the line', () => assert.include(errorOutput, 'on line 3')); - it('does not contain any NaNs', () => assert.notInclude(errorOutput, 'nan')); - }); - }); - - describe('Prints parser warning', () => { - let error; - - const apiDescriptionDocument = `\ -FORMAT: 1A -# Beehive API -## Honey [/honey] -### Remove [DELETE] -+ Response\ -`; - const filename = 'dummy-filename.apib'; - - beforeEach(done => prepareData(apiDescriptionDocument, filename, (err, data) => { - if (err) { return done(err); } - error = handleRuntimeProblems(data); - done(); - })); - - it('returns no error', () => assert.notOk(error)); - it('has no error output', () => assert.equal(errorOutput, '')); - it('has warning output', () => assert.isOk(warnOutput)); - context('the warning output', () => { - it('mentions it is from parser', () => assert.include(warnOutput, 'parser')); - it('mentions it is warning', () => assert.include(warnOutput, 'warn')); - it('mentions the filename', () => assert.include(warnOutput, filename)); - it('mentions the line', () => assert.include(warnOutput, 'on line 5')); - it('does not contain any NaNs', () => assert.notInclude(warnOutput, 'nan')); - }); - }); - - describe('Prints warning about missing title', () => { - let error; - - const apiDescriptionDocument = `\ -FORMAT: 1A -So Long, and Thanks for All the Fish!\ -`; - const filename = 'dummy-filename.apib'; - - beforeEach(done => prepareData(apiDescriptionDocument, filename, (err, data) => { - if (err) { return done(err); } - error = handleRuntimeProblems(data); - done(); - })); - - it('returns no error', () => assert.notOk(error)); - it('has no error output', () => assert.equal(errorOutput, '')); - it('has warning output', () => assert.isOk(warnOutput)); - context('the warning output', () => { - it('mentions it is from parser', () => assert.include(warnOutput, 'parser')); - it('mentions it is warning', () => assert.include(warnOutput, 'warning')); - it('mentions the filename', () => assert.include(warnOutput, filename)); - it('mentions the line', () => assert.include(warnOutput, 'on line 1')); - it('does not contain any NaNs', () => assert.notInclude(warnOutput, 'nan')); - }); - }); -}); From 9513b9a518b8d78773b4e57fd4f7ad0d382c2727 Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Mon, 6 May 2019 17:19:18 +0200 Subject: [PATCH 2/3] feat: bring in (not only) OAS3 fixes and features Please see https://github.com/apiaryio/dredd-transactions/releases and https://github.com/apiaryio/api-elements.js/releases for more details Close https://github.com/apiaryio/dredd/pull/1338 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0c62b42cf..4c4cc8a87 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "chai": "4.2.0", "clone": "2.1.2", "cross-spawn": "6.0.5", - "dredd-transactions": "7.0.0", + "dredd-transactions": "8.1.3", "gavel": "3.0.1", "glob": "7.1.3", "html": "1.0.0", From 8e9f68a78bd87cddf867ae7a3870b12937e5a715 Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Fri, 10 May 2019 11:16:58 +0200 Subject: [PATCH 3/3] style: add link to the Winston 3 issue --- lib/Dredd.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Dredd.js b/lib/Dredd.js index ff3968011..02081adab 100644 --- a/lib/Dredd.js +++ b/lib/Dredd.js @@ -178,6 +178,8 @@ class Dredd { // FIXME: Winston 3.x supports calling .log() directly with the loggerInfo // object as it's sole argument, but that's not the case with Winston 2.x // Once we upgrade Winston, the line below can be simplified to .log(loggerInfo) + // + // Watch https://github.com/apiaryio/dredd/issues/1225 for updates loggerInfos.forEach(({ level, message }) => this.logger.log(level, message)); if (loggerInfos.find(loggerInfo => loggerInfo.level === 'error')) { callback(new Error('API description processing error'), this.stats);