processing / processing/p5.js

p5.Vector: mock setup blocks the 8 disabled rotate tests — 6 revivable, 2 need a decision

Open
#9,144 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Discussion
Dominant language
JavaScript
Stars
24k
Forks
3.8k
Avg merge
3d 16h
Merged PRs (30d)
25

Description

Topic

Follow-up to #9139 — same origin commit, different outcome.

Commit 7af496704 ("Mark most failing tests as todos", Sep 2024) disabled eight
rotate tests in test/unit/math/p5.Vector.js (suite.todo at lines 189, 219, 242).
Unlike the p5.Color ones in #9139, these can't just be re-enabled. I traced the
blockers and wanted to flag what I found rather than guess at the right fix.

Two errors, in sequence

Re-enabling the suites as-is gives:

ReferenceError: RADIANS is not defined
  test/unit/math/p5.Vector.js:191
    mockP5Prototype.angleMode(RADIANS);

RADIANS/DEGREES live in src/math/trigonometry.js (line 87: const RADIANS = (fn.RADIANS = 'radians')), attached to whatever object the module is
initialised with. The test file calls math() and vector() in beforeAll but
never trigonometry(), so angleMode and the constants don't exist on the mock.
The 2024 diff shows these calls were originally mockP5.RADIANS; the prefix
was dropped at some point and the bare identifiers were never imported —
invisible because the suites never ran.

Adding trigonometry(mockP5, mockP5Prototype) and qualifying the constants
clears that, and surfaces the next one:

TypeError: Right-hand side of 'instanceof' is not callable
  src/math/math.js:95
    if (this instanceof p5) {

mockP5 in this file is an object literal, and instanceof requires a
function on the right. (test/js/mocks.js uses vi.fn() for its mock, which is
presumably why p5.Color's tests work.)

What revives 6 of the 8

Three changes, all in the test file:

  1. Import and call trigonometry(mockP5, mockP5Prototype) in beforeAll
  2. Change const mockP5 = { ... } to vi.fn() with _friendlyError assigned
    afterwards
  3. Reference the constants as mockP5Prototype.RADIANS / .DEGREES

Result: the three radians tests and all three [CLASS] tests pass. The two
degrees tests still fail.

Why the last two fail

createVector only binds the angle-conversion helpers inside the
this instanceof p5 branch. mockP5Prototype isn't an instance of mockP5,
so that branch never runs and degree→radian conversion is never wired up.
Radians pass because that's the default no-op path.

Evidence: rotating [0,1,0] by 180 in DEGREES mode gives x = 0.801, which is
|sin(180 radians)|. The 180 is being treated as radians.

The question

Should the mock be constructed so this instanceof p5 holds — which would
also unblock the p5.prototype.createVector / setHeading todos and the
angleMode(DEGREES) heading tests, all of which depend on the same path — or
should the degree-mode tests stay disabled and be covered elsewhere?

Happy to implement whichever direction is preferred. I haven't opened a PR.

Environment: macOS, Node v22.22.2, Vitest 4.1.10, chromium.

Contributor guide

Open the contributing guide

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.

Research direction

Start with the disabled suites in test/unit/math/p5.Vector.js and the mock initialization in test/js/mocks.js; run the rotate tests to reproduce the RADIANS and instanceof failures. Read src/math/trigonometry.js and src/math/math.js to understand the required mock setup. Done means the six identified tests pass and the project has a decided, documented outcome for the two degree-mode tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.