`test/modules/powSqrt.js` has never run — `ReferenceError: total is not defined`

Open Beginner friendly
#262 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
javascript
Domain
testing-qa

Research direction

Start with test/modules/powSqrt.js and reproduce the ReferenceError using the node command in the issue. Read test/setup.js to understand the available counters and test/test.js to check how modules are listed. Done means powSqrt runs without the undefined variable error, is included in the suite, and its assertions pass.

Written by the indexing model from the issue text.

Description

Summary

test/modules/powSqrt.js cannot execute. It throws before its first assertion,
on a clean checkout of master, on any Node version.

Reproduction
git clone https://github.com/MikeMcl/decimal.js
cd decimal.js
node -e "require('./test/modules/powSqrt.js')"
 Testing pow against sqrt...
ReferenceError: total is not defined
    at .../test/modules/powSqrt.js:12
Cause

Line 12 loops on a free variable:

for (var e, n, p, r, s; total < 10000; ) {

Nothing defines total. test/setup.js keeps its counters as closure variables
inside Tpassed and testNumber — and exposes them only afterwards, as
T.result. There is no global of that name, so the comparison throws on the
first evaluation of the loop condition.

test/test.js computes a local total while summing results, which looks like
where the name came from, but that variable is not in scope here and the module
is required in its own right.

Why it went unnoticed

test/test.js lists 60 modules to require, and powSqrt is not among them —
test/modules/ holds 61 files. So npm test never loads it, and the failure
never surfaces.

Why it matters

The module is a genuinely valuable cross-check that is currently doing nothing.
It compares r.pow(0.5) against r.sqrt() for random values, at a random
rounding mode and a random precision in [1, 40], ten thousand times — which
exercises naturalExponential and naturalLogarithm against the independent
Newton-Raphson path in squareRoot. Nothing else in the suite pits those two
implementations against each other.

Suggested fixes

Either would do:

  1. Use the harness's own counter. T.result is only set once the module
    finishes, so this needs a live counter — e.g. exposing testNumber from
    setup.js, and looping on that.

  2. Loop a fixed number of times, which is what the code appears to intend:

for (var e, n, p, r, s, i = 0; i < 10000; i++) {

Then add 'powSqrt' to the module list in test/test.js so it actually runs.
Be aware it is slow — ten thousand pow(0.5) calls at up to 40 significant
digits.

Dominant language
JavaScript
Stars
7.3k
Forks
498
Avg merge
17h 52m
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MikeMcl/decimal.js

All issues in MikeMcl/decimal.js

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.