Skip to content

Conversation

@ibgreen
Copy link
Contributor

@ibgreen ibgreen commented Dec 18, 2025

Summary

  • ensure explicit benchmark iterations run fixed, single-pass measurements instead of adaptive time-based loops and cover the behavior in tests
  • document how the iterations option now skips adaptive timing by default and how to opt back in
  • cap the GitHub Actions test job to a 10 minute timeout to prevent runaway CI runs

Testing

  • yarn test bench (fails: missing @probe.gl/test-utils dependency in the local environment)

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +52 to +54
suite.run().then(() => {
t.equals(callCount, 2, 'runs configured number of iterations');
t.end();

Choose a reason for hiding this comment

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

P1 Badge Account for calibration pass when asserting iteration counts

These new assertions assume the benchmarked test runs exactly the configured iterations (2 here and 1 in the later test), but runTests still performs a calibration warm‑up via runCalibrationTests before the measured run. That warm‑up invokes each test once, so callCount will be 3 in this test and 2 in the single‑iteration case, causing yarn test bench to fail and the iteration option to execute more times than the test claims.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants