Skip to content

Commit 3166b7f

Browse files
josdejonggwhitney
andauthored
Gwhitney doctesting (#2471)
* test: Add unit tests for all of the examples in (jsdoc) comments Uses the existing extraction of examples from tools/docgenerator.js Hence, for now this is limited to documentation of functions, but hopefully it can be extended to classes, units (and physical constants), and constants as well in the future. Exposes numerous errors in the examples, some of which are bugs; these are for now put on a known error list to be worked on, so that this PR does not change a huge number of source files. Also adds a test to check that all symbols are documented (which similarly doesn't really pass at the moment, and is patched to a hopefully temporary warning). * refactor: Make doc.test.js into a node test The source code is not available in its layout as in the repository in the browser tests, so the new doc testing can only occur in the node tests * Add simplifyCore, symbolicEqual, map, and resolve to the list with functions with known issues in the jsdoc examples Co-authored-by: Glen Whitney <[email protected]>
1 parent 8f9624b commit 3166b7f

File tree

2 files changed

+535
-137
lines changed

2 files changed

+535
-137
lines changed

test/node-tests/doc.test.js

Lines changed: 386 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,386 @@
1+
const assert = require('assert')
2+
const path = require('path')
3+
4+
const approx = require('../../tools/approx.js')
5+
const docgenerator = require('../../tools/docgenerator.js')
6+
const math = require('../..')
7+
8+
function extractExpectation (comment, optional = false) {
9+
if (comment === '') return undefined
10+
const returnsParts = comment.split('eturns').map(s => s.trim())
11+
if (returnsParts.length > 1) return extractValue(returnsParts[1])
12+
const outputsParts = comment.split('utputs')
13+
if (outputsParts.length > 1) {
14+
let output = outputsParts[1]
15+
if (output[0] === ':') output = output.substring(1)
16+
return extractValue(output.trim())
17+
}
18+
// None of the usual flags; if we need a value,
19+
// assume the whole comment is the desired value. Otherwise return undefined
20+
if (optional) return undefined
21+
return extractValue(comment)
22+
}
23+
24+
function extractValue (spec) {
25+
// First check for a leading keyword:
26+
const words = spec.split(' ')
27+
// If the last word end in 'i' and the value is not labeled as complex,
28+
// label it for the comment writer:
29+
if (words[words.length - 1].substr(-1) === 'i' && words[0] !== 'Complex') {
30+
words.unshift('Complex')
31+
}
32+
// Collapse 'Dense Matrix' into 'DenseMatrix'
33+
if (words[0] === 'Dense' && words[1] === 'Matrix') {
34+
words.shift()
35+
words[0] = 'DenseMatrix'
36+
}
37+
const keywords = {
38+
number: 'Number(_)',
39+
BigNumber: 'math.bignumber(_)',
40+
Fraction: 'math.fraction(_)',
41+
Complex: "math.complex('_')",
42+
Unit: "math.unit('_')",
43+
Array: '_',
44+
Matrix: 'math.matrix(_)',
45+
DenseMatrix: "math.matrix(_, 'dense')",
46+
string: '_',
47+
Node: 'math.parse(_)',
48+
throws: "'_'"
49+
}
50+
if (words[0] in keywords) {
51+
const template = keywords[words[0]]
52+
const spot = template.indexOf('_')
53+
let filler = words.slice(1).join(' ')
54+
if (words[0] === 'Complex') { // a bit of a hack here :(
55+
filler = words.slice(1).join('')
56+
}
57+
spec = template.substring(0, spot) + filler + template.substr(spot + 1)
58+
}
59+
if (spec.substring(0, 7) === 'matrix(') {
60+
spec = 'math.' + spec // More hackery :(
61+
}
62+
let value
63+
try {
64+
value = eval(spec) // eslint-disable-line no-eval
65+
} catch (err) {
66+
if (err instanceof SyntaxError || err instanceof ReferenceError) {
67+
value = spec
68+
} else {
69+
throw err
70+
}
71+
}
72+
if (words[0] === 'Unit') { // more hackishness here :(
73+
value.fixPrefix = true
74+
}
75+
if (words[0] === 'Node') { // and even more :(
76+
delete value.comment
77+
}
78+
return value
79+
}
80+
81+
const knownProblems = new Set([
82+
'numeric', 'isZero', 'isPositive', 'isNumeric', 'isNegative', 'isNaN',
83+
'isInteger', 'hasNumericValue', 'clone', 'print', 'hex', 'format', 'to', 'sin',
84+
'cos', 'atan2', 'atan', 'asin', 'asec', 'acsc', 'acoth', 'acot', 'max',
85+
'setUnion', 'unequal', 'equal', 'deepEqual', 'compareNatural', 'randomInt',
86+
'random', 'pickRandom', 'kldivergence', 'xor', 'or', 'not', 'and', 'distance',
87+
'parser', 'compile', 're', 'im', 'rightLogShift', 'rightArithShift',
88+
'leftShift', 'bitNot', 'apply', 'subset', 'squeeze', 'rotationMatrix',
89+
'rotate', 'reshape', 'partitionSelect', 'matrixFromRows', 'matrixFromFunction',
90+
'matrixFromColumns', 'getMatrixDataType', 'forEach', 'eigs', 'diff',
91+
'ctranspose', 'concat', 'sqrtm', 'subtract', 'nthRoots', 'nthRoot', 'multiply',
92+
'mod', 'invmod', 'floor', 'fix', 'expm1', 'exp', 'dotPow', 'dotMultiply',
93+
'dotDivide', 'divide', 'ceil', 'cbrt', 'add', 'usolveAll', 'usolve', 'slu',
94+
'rationalize', 'qr', 'lusolve', 'lup', 'lsolveAll', 'lsolve', 'derivative',
95+
'simplifyCore', 'symbolicEqual', 'map', 'resolve'
96+
])
97+
98+
function maybeCheckExpectation (name, expected, expectedFrom, got, gotFrom) {
99+
if (knownProblems.has(name)) {
100+
try {
101+
checkExpectation(expected, got)
102+
} catch (err) {
103+
console.log(
104+
`PLEASE RESOLVE: '${gotFrom}' was supposed to '${expectedFrom}'`)
105+
console.log(' but', err.toString())
106+
}
107+
} else {
108+
checkExpectation(expected, got)
109+
}
110+
}
111+
112+
function checkExpectation (want, got) {
113+
if (Array.isArray(want) && !Array.isArray(got)) {
114+
approx.deepEqual(got, math.matrix(want), 1e-9)
115+
} else if (want instanceof math.Unit && got instanceof math.Unit) {
116+
approx.deepEqual(got, want, 1e-9)
117+
} else if (want instanceof math.Complex && got instanceof math.Complex) {
118+
approx.deepEqual(got, want, 1e-9)
119+
} else if (typeof want === 'number' &&
120+
typeof got === 'number' &&
121+
want !== got) {
122+
approx.equal(got, want, 1e-9)
123+
console.log(` Note: return value ${got} not exactly as expected: ${want}`)
124+
} else {
125+
assert.deepEqual(got, want)
126+
}
127+
}
128+
129+
const OKundocumented = new Set([
130+
'addScalar', 'divideScalar', 'multiplyScalar', 'equalScalar',
131+
'docs', 'FibonacciHeap',
132+
'IndexError', 'DimensionError', 'ArgumentsError'
133+
])
134+
135+
const knownUndocumented = new Set([
136+
'all',
137+
'isNumber',
138+
'isComplex',
139+
'isBigNumber',
140+
'isFraction',
141+
'isUnit',
142+
'isString',
143+
'isArray',
144+
'isMatrix',
145+
'isCollection',
146+
'isDenseMatrix',
147+
'isSparseMatrix',
148+
'isRange',
149+
'isIndex',
150+
'isBoolean',
151+
'isResultSet',
152+
'isHelp',
153+
'isFunction',
154+
'isDate',
155+
'isRegExp',
156+
'isObject',
157+
'isNull',
158+
'isUndefined',
159+
'isAccessorNode',
160+
'isArrayNode',
161+
'isAssignmentNode',
162+
'isBlockNode',
163+
'isConditionalNode',
164+
'isConstantNode',
165+
'isFunctionAssignmentNode',
166+
'isFunctionNode',
167+
'isIndexNode',
168+
'isNode',
169+
'isObjectNode',
170+
'isOperatorNode',
171+
'isParenthesisNode',
172+
'isRangeNode',
173+
'isSymbolNode',
174+
'isChain',
175+
'on',
176+
'off',
177+
'once',
178+
'emit',
179+
'config',
180+
'expression',
181+
'import',
182+
'create',
183+
'factory',
184+
'AccessorNode',
185+
'ArrayNode',
186+
'AssignmentNode',
187+
'atomicMass',
188+
'avogadro',
189+
'BigNumber',
190+
'bignumber',
191+
'BlockNode',
192+
'bohrMagneton',
193+
'bohrRadius',
194+
'boltzmann',
195+
'boolean',
196+
'chain',
197+
'Chain',
198+
'classicalElectronRadius',
199+
'complex',
200+
'Complex',
201+
'ConditionalNode',
202+
'conductanceQuantum',
203+
'ConstantNode',
204+
'coulomb',
205+
'createUnit',
206+
'DenseMatrix',
207+
'deuteronMass',
208+
'e',
209+
'efimovFactor',
210+
'electricConstant',
211+
'electronMass',
212+
'elementaryCharge',
213+
'false',
214+
'faraday',
215+
'fermiCoupling',
216+
'fineStructure',
217+
'firstRadiation',
218+
'fraction',
219+
'Fraction',
220+
'FunctionAssignmentNode',
221+
'FunctionNode',
222+
'gasConstant',
223+
'gravitationConstant',
224+
'gravity',
225+
'hartreeEnergy',
226+
'Help',
227+
'i',
228+
'ImmutableDenseMatrix',
229+
'index',
230+
'Index',
231+
'IndexNode',
232+
'Infinity',
233+
'inverseConductanceQuantum',
234+
'klitzing',
235+
'LN10',
236+
'LN2',
237+
'LOG10E',
238+
'LOG2E',
239+
'loschmidt',
240+
'magneticConstant',
241+
'magneticFluxQuantum',
242+
'matrix',
243+
'Matrix',
244+
'molarMass',
245+
'molarMassC12',
246+
'molarPlanckConstant',
247+
'molarVolume',
248+
'NaN',
249+
'neutronMass',
250+
'Node',
251+
'nuclearMagneton',
252+
'null',
253+
'number',
254+
'ObjectNode',
255+
'OperatorNode',
256+
'ParenthesisNode',
257+
'parse',
258+
'Parser',
259+
'phi',
260+
'pi',
261+
'planckCharge',
262+
'planckConstant',
263+
'planckLength',
264+
'planckMass',
265+
'planckTemperature',
266+
'planckTime',
267+
'protonMass',
268+
'quantumOfCirculation',
269+
'Range',
270+
'RangeNode',
271+
'reducedPlanckConstant',
272+
'RelationalNode',
273+
'replacer',
274+
'ResultSet',
275+
'reviver',
276+
'rydberg',
277+
'SQRT1_2',
278+
'SQRT2',
279+
'sackurTetrode',
280+
'secondRadiation',
281+
'Spa',
282+
'sparse',
283+
'SparseMatrix',
284+
'speedOfLight',
285+
'splitUnit',
286+
'stefanBoltzmann',
287+
'string',
288+
'SymbolNode',
289+
'tau',
290+
'thomsonCrossSection',
291+
'true',
292+
'typed',
293+
'Unit',
294+
'unit',
295+
'E',
296+
'PI',
297+
'vacuumImpedance',
298+
'version',
299+
'weakMixingAngle',
300+
'wienDisplacement'
301+
])
302+
303+
const bigwarning = `WARNING: ${knownProblems.size} known errors converted ` +
304+
'to PLEASE RESOLVE warnings.' +
305+
`\n WARNING: ${knownUndocumented.size} symbols in math are known to ` +
306+
'be undocumented; PLEASE EXTEND the documentation.'
307+
308+
describe(bigwarning + '\n Testing examples from (jsdoc) comments', () => {
309+
const allNames = Object.keys(math)
310+
const srcPath = path.resolve(__dirname, '../../src') + '/'
311+
const allDocs = docgenerator.collectDocs(allNames, srcPath)
312+
it("should cover all names (but doesn't yet)", () => {
313+
const documented = new Set(Object.keys(allDocs))
314+
const badUndocumented = allNames.filter(name => {
315+
return !(documented.has(name) ||
316+
OKundocumented.has(name) ||
317+
knownUndocumented.has(name) ||
318+
name.substr(0, 1) === '_' ||
319+
name.substr(-12) === 'Dependencies' ||
320+
name.substr(0, 6) === 'create'
321+
)
322+
})
323+
assert.deepEqual(badUndocumented, [])
324+
})
325+
const byCategory = {}
326+
for (const fun of Object.values(allDocs)) {
327+
if (!(fun.category in byCategory)) {
328+
byCategory[fun.category] = []
329+
}
330+
byCategory[fun.category].push(fun.doc)
331+
}
332+
for (const category in byCategory) {
333+
describe('category: ' + category, () => {
334+
for (const doc of byCategory[category]) {
335+
it('satisfies ' + doc.name, () => {
336+
console.log(` Testing ${doc.name} ...`) // can remove once no known failures; for now it clarifies "PLEASE RESOLVE"
337+
const lines = doc.examples
338+
lines.push('//') // modifies doc but OK for test
339+
let accumulation = ''
340+
let expectation
341+
let expectationFrom = ''
342+
for (const line of lines) {
343+
if (line.includes('//')) {
344+
let parts = line.split('//')
345+
if (parts[0] && !parts[0].trim()) {
346+
// Indented comment, unusual in examples
347+
// assume this is a comment within some code to evaluate
348+
// i.e., ignore it
349+
continue
350+
}
351+
// Comment specifying a future value or the return of prior code
352+
parts = parts.map(s => s.trim())
353+
if (parts[0] !== '') {
354+
if (accumulation) { accumulation += '\n' }
355+
accumulation += parts[0]
356+
}
357+
if (accumulation !== '' && expectation === undefined) {
358+
expectationFrom = parts[1]
359+
expectation = extractExpectation(expectationFrom)
360+
parts[1] = ''
361+
}
362+
if (accumulation) {
363+
let value
364+
try {
365+
value = eval(accumulation) // eslint-disable-line no-eval
366+
} catch (err) {
367+
value = err.toString()
368+
}
369+
maybeCheckExpectation(
370+
doc.name, expectation, expectationFrom, value, accumulation)
371+
accumulation = ''
372+
}
373+
expectationFrom = parts[1]
374+
expectation = extractExpectation(expectationFrom, 'requireSignal')
375+
} else {
376+
if (line !== '') {
377+
if (accumulation) { accumulation += '\n' }
378+
accumulation += line
379+
}
380+
}
381+
}
382+
})
383+
}
384+
})
385+
}
386+
})

0 commit comments

Comments
 (0)