p5.Vector: mock setup blocks the 8 disabled rotate tests — 6 revivable, 2 need a decision
Nobody has claimed this yet.
- 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:
- Import and call
trigonometry(mockP5, mockP5Prototype)in beforeAll - Change
const mockP5 = { ... }tovi.fn()with_friendlyErrorassigned
afterwards - 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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