Skip to content

Add unit tests for isLooping() default, noLoop() and loop() states#8981

Open
harshiltewari2004 wants to merge 1 commit into
processing:mainfrom
harshiltewari2004:islooping-tests
Open

Add unit tests for isLooping() default, noLoop() and loop() states#8981
harshiltewari2004 wants to merge 1 commit into
processing:mainfrom
harshiltewari2004:islooping-tests

Conversation

@harshiltewari2004

@harshiltewari2004 harshiltewari2004 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Resolves #8979

Changes

  • Added a p5.prototype.isLooping suite to test/unit/core/structure.js covering the three states scoped in the issue: true by default, false after noLoop(), true again after loop().
  • The suite creates its own p5 instance instead of using the file's shared one. Toggling noLoop()/loop() within the same frame on the shared instance double-pumps its draw loop — noLoop() doesn't cancel the pending requestAnimationFrame tick, and loop() immediately starts a new chain, so the stale tick resurrects alongside it. This made the existing timing-based "noLoop should stop" test fail (reproduced consistently; passes consistently without the new suite). A private instance isolates the toggling completely.
  • The tests are synchronous, unlike the neighboring loop/noLoop tests: The tests are synchronous, unlike the neighboring loop/noLoop tests: those verify the draw loop's actual behavior, which needs real time to elapse so pending requestAnimationFrame ticks can fire (or fail to). These tests only assert the state flag, which noLoop()/loop() set synchronously before returning, so there's nothing to wait for.

PR Checklist

  • npm run lint passes
  • [Inline reference] is included / updated — N/A, test-only change
  • [Unit tests] are included / updated

@lirenjie95 lirenjie95 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the placeholder in the PR description is cleaned up.

@harshiltewari2004

Copy link
Copy Markdown
Contributor Author

Cleaned up-thanks for the catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[p5.js 2.0+ Bug Report]: Missing unit tests for isLooping() return value

2 participants