Skip to content

adopt vitest; fix hex bug#2373

Open
Fil wants to merge 8 commits intomainfrom
fil/vitest
Open

adopt vitest; fix hex bug#2373
Fil wants to merge 8 commits intomainfrom
fil/vitest

Conversation

@Fil
Copy link
Contributor

@Fil Fil commented Mar 4, 2026

This:

  • replaces mocha by vitest as the preferred test framework
  • defaults offset to 0 (high-res screen by default)
  • fixes an offset inconsistency in the hex marks

AI disclosure: I used @claude to generate this PR. Reviewed every line very carefully though.

Fil added 5 commits March 3, 2026 15:10
…ndefined (older browser), or <= 1 (actual low-res screen)
- removes translate(0, 0.5)
- uses lower-precision numbers (#2270, #2234)

closes #1445
@Fil Fil requested a review from mbostock March 4, 2026 20:30
test/plot.js Outdated
@@ -56,7 +63,7 @@ for (const [name, plot] of Object.entries(plots)) {
await fs.writeFile(diffile, actual, "utf8");
}

assert.ok(equal, `${name} must match snapshot`);
expect(equal, `${name} must match snapshot`).toBe(true);
Copy link
Member

Choose a reason for hiding this comment

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

Don’t we want to use Vitest’s file snapshots here? In my mind that was one of the primary goals of adopting Vitest. See (internal) example: https://github.com/observablehq/charts/blob/main/test/charts/test.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue here is that snapshots have embedded images and that the file snapshots need to discard them (with something like <replaced>) because of inconsistencies between platforms. We can adopt a system where the images are compared separately, but it's not so great when you review changes visually.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can do it with a custom snapshot serializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New strategy: extract the images to the side as PNGs, apply the image comparison algo to them, and link them by name instead of the <replaced> string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I understand it a custom serializer would create the output as a .snap file containing the SVG/HTML structure and the images. But the comparison would still want an exact equality, which can't be guaranteed across platforms. We need the comparison to say "ok" even when files are slightly different, but I think I found a way to do this by doing the substitution early if the image discrepancy is low.

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.

2 participants