From 8e41fd728db558f8fb464c257349a28ab61534f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=BA=D0=BE=D0=B2=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20?= =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0=20=D0=90=D0=BD=D0=B4=D1=80?= =?UTF-8?q?=D0=B5=D0=B5=D0=B2=D0=B8=D1=87?= Date: Fri, 22 Jun 2018 20:16:41 +0300 Subject: [PATCH] test: add test for error codes doc and impl This adds several sanity checks for error codes. It scans: * all natives (js sources), * doc/api/*.md documentation * src/node_errors.h (errors definition from the C++ side). There is also a whitelist of manually created errors from JS side, currently consisting of ERR_HTTP2_ERROR and ERR_UNKNOWN_BUILTIN_MODULE. Alsom all ERR_NAPI_ codes are whitelisted, as those are created directly on the cpp side, without declaring them first. The performed checks: 1. All errors used from JS should be defined in `internal/errors` and present in its .codes object. Whitelist (mentioned above) applies. 2. All errors instantiated from JS without arguments should support 0-arguments version. 3. All errors mentioned in doc should defined either in JS, C++, or in the whitelist. 4. All errors mentioned anywhere should be documented. 5. Documentation of error codes should be sorted, have no repeats, and include exactly one entry for every error code mentioned in the documentation, formatted as `/\n### (ERR_[A-Z0-9_]+)\n`. 6. All doc entries for error codes should have appropriate anchors. There is also a --report flag, which prints all the current issues and exits without asserting, for manual inspection. Individual fixes for those issues are landed in separate commits. Refs: https://github.com/nodejs/node/issues/21421 Refs: https://github.com/nodejs/node/issues/21440 Refs: https://github.com/nodejs/node/pull/21483 Refs: https://github.com/nodejs/node/pull/21484 Refs: https://github.com/nodejs/node/pull/21485 Refs: https://github.com/nodejs/node/pull/21487 PR-URL: https://github.com/nodejs/node/pull/21470 --- test/parallel/test-error-codes-impl-doc.js | 158 +++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 test/parallel/test-error-codes-impl-doc.js diff --git a/test/parallel/test-error-codes-impl-doc.js b/test/parallel/test-error-codes-impl-doc.js new file mode 100644 index 00000000000000..8ec0fb73fb7c78 --- /dev/null +++ b/test/parallel/test-error-codes-impl-doc.js @@ -0,0 +1,158 @@ +'use strict'; + +// Flags: --expose-internals + +// Note: this script reads Node.js sources and docs, and expects to be run on +// the very same Node.js version as the source and doc tree is. + +require('../common'); +const assert = require('assert'); +const errors = require('internal/errors').codes; +const fs = require('fs'); +const path = require('path'); +const natives = process.binding('natives'); + +// --report prints only the list of failed checks and does not assert +const report = process.argv[2] === '--report'; +const results = []; +function check(ok, type, name, reason) { + const label = `${type}: ${name} - ${reason}`; + if (report) { + if (!ok) results.push(label); + } else { + assert.ok(ok, label); + } +} + +const set = { + all: new Set(), + js: new Set(), // Error codes found in natives. + doc: new Set(), // Error codes mentioned in `doc/api/*.md`. + cpp: new Set(), // Error codes found in `src/node_errors.h`. + noargs: new Set(), // Subset of `js` which is constructed without arguments. + // `set.manual` contains errors manually created from js and not defined in + // internals/errors. That is not scanned now, update this list if required. + manual: new Set(['ERR_HTTP2_ERROR', 'ERR_UNKNOWN_BUILTIN_MODULE']), + // `set.examples` contains errors mentioned in documentation purely for + // demonstration purposes. These errors are not required to be present. + examples: new Set(['ERR_ERROR_1']) +}; + +const root = path.join(__dirname, '..', '..'); + +// File containing error definitions +const jsdata = natives['internal/errors']; +// File containing error documentation +let docdata = fs.readFileSync(path.join(root, 'doc/api/errors.md'), 'utf-8') +docdata = docdata.replace(/## Legacy [\s\S]*/, ''); +// File containing cpp-side errors +const cppdata = fs.readFileSync(path.join(root, 'src/node_errors.h'), 'utf-8'); +// Directory containing documentation +const docdir = path.join(root, 'doc/api/'); + +function addSource(source, type) { + // eslint-disable-next-line node-core/no-unescaped-regexp-dot + const re = /(.)?(ERR_[A-Z0-9_]+)(..)?/g; + let match; + while (match = re.exec(source)) { + if (match[1] === '_') continue; // does not start with ERR_ + const name = match[2]; + if (type === 'doc' && set.examples.has(name)) continue; // is an example + set.all.add(name); + set[type].add(name); + if (type === 'js' && match[3] === '()') { + // Is directly called without arguments from js, we will try that + set.noargs.add(name); + } + } +} + +// Add all errors from JS natives +for (const file of Object.keys(natives)) addSource(natives[file], 'js'); +// Add all errors from src/node_errors.h +addSource(cppdata, 'cpp'); +// Add all errors from doc/api/*.md files +for (const file of fs.readdirSync(docdir)) { + if (!file.endsWith('.md')) continue; + let data = fs.readFileSync(path.join(docdir, file), 'utf-8'); + if (file === 'errors.md') data = data.replace(/## Legacy [\s\S]*/, ''); + addSource(data, 'doc'); +} + +// Check that we have all js errors +for (const name of set.js) { + if (set.manual.has(name)) continue; + const defined = jsdata.includes(`E('${name}',`); + check(defined, 'js', name, 'missing JS implementation (source)'); + if (defined) { + check(errors[name], 'js', name, 'missing JS implementation (runtime)'); + } +} + +// Check that we can initialize errors called without args +for (const name of set.noargs) { + if (!errors[name]) continue; // Already catched that above + let ok = true; + try { + new errors[name](); + } catch (e) { + ok = false; + } + check(ok, 'init', name, 'failed init without args, but is called with "()"'); +} + +// Check that we have implementation for all errors mentioned in docs. +// C++ does not need that, and JS is already checked above. +for (const name of set.doc) { + const ok = set.manual.has(name) || + name.startsWith('ERR_NAPI_') || // napi errors are created directly + jsdata.includes(`E('${name}',`) || + cppdata.includes(`V(${name}, `); + const reason = docdata.includes(`### ${name}\n`) ? + 'documented' : 'mentioned in doc/api/'; + check(ok, 'impl', name, `missing implementation, ${reason}`); +} + +// Check that we have documentation for all errors +for (const name of set.all) { + const ok = docdata.includes(`### ${name}\n`); + check(ok, 'doc', name, 'missing documentation'); + // Check that documentation references are correctly formatted + if (ok) { + const anchor = docdata.includes(`\n\n\n### ${name}\n`); + check(anchor, 'doc', name, 'missing anchor or not properly formatted'); + } +} + +// Check that documentation is sorted, formatted, and does not contain dupes +{ + const compare = (a, b) => { + // HTTP2_ should come after HTTP_ + if (a.startsWith('ERR_HTTP_') && b.startsWith('ERR_HTTP2_')) return -1; + if (a.startsWith('ERR_HTTP2_') && b.startsWith('ERR_HTTP_')) return 1; + if (a < b) return -1; + if (a > b) return 1; + return 0; + }; + const re = /\n### (ERR_[A-Z0-9_]+)\n/g; + let match; + let last; + const documented = new Set(); + while (match = re.exec(docdata)) { + const name = match[1]; + if (documented.has(name)) { + check(false, 'doc', name, 'duplicate documentation entry'); + } else { + const sorted = !last || compare(name, last) === 1; + check(sorted, 'doc', name, 'is out of order'); + documented.add(name); + } + last = name; + } +} + +if (report) { + console.log('Report mode'); + console.log(results.join('\n')); + console.log(`There were ${results.length} problems found`); +}