Skip to content

tests(mvt): validate Tippecanoe TileJSON parsing, skip indexed tilestats, and enforce maxValues truncation#3266

Open
ibgreen wants to merge 2 commits intomasterfrom
codex/enhance-tilejson-loader-tests
Open

tests(mvt): validate Tippecanoe TileJSON parsing, skip indexed tilestats, and enforce maxValues truncation#3266
ibgreen wants to merge 2 commits intomasterfrom
codex/enhance-tilejson-loader-tests

Conversation

@ibgreen
Copy link
Copy Markdown
Collaborator

@ibgreen ibgreen commented Dec 19, 2025

Motivation

  • Improve TileJSON test coverage for Tippecanoe-generated metadata by asserting the full parsed metadata, not just the presence of layers.
  • Ensure indexed tilestats attributes (e.g. names containing |) are ignored by the parser and do not surface as fields.
  • Verify that the tilejson.maxValues option truncates fields[].values as expected.

Description

  • Updated test modules/mvt/test/tilejson-loader.spec.ts to load the expected parsed TileJSON via JSONLoader and assert deep equality with the parsed output from TileJSONLoader.
  • Added assertions that indexed tilestats attributes are skipped, tilestats properties (like dominantGeometry and minZoom) are merged, and that maxValues truncates values correctly.
  • Extended the input fixture modules/mvt/test/data/tilejson/tippecanoe.tilejson to include an indexed tilestats attribute (e.g. scalerank|0) so the code path that ignores indexed attributes is exercised.
  • Refreshed the expected parsed output fixture modules/mvt/test/data/tilejson/tippecanoe.expected.json to match the parsed/normalized TileJSON structure used in assertions.

Testing

  • Attempted to run formatting via yarn lint fix (required by project guidelines); it failed due to missing node_modules state file in the environment.
  • Attempted to run unit tests via yarn test node; this also failed for the same missing node_modules state file.
  • The changes are limited to test fixtures and tests; they are ready for CI where dependencies are available and the test suite can be executed.

Codex Task

Copy link
Copy Markdown

@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".

const fields = metadata.layers?.[0]?.fields || [];
const indexedFieldNames = fields.filter((field) => field.name.includes('|'));
t.equal(indexedFieldNames.length, 0, 'Indexed tilestats attributes are skipped');
t.equal(metadata.layers?.[0]?.minZoom, 0, 'Vector layer minZoom is merged');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Assert minZoom only if parser normalizes minzoom

This assertion will fail with the current TileJSON parser because parseTileJSONLayer preserves the original minzoom key from vector_layers and never remaps it to minZoom. As a result, metadata.layers?.[0]?.minZoom is undefined for Tippecanoe input (which uses minzoom), so the new test (and the updated expected fixture that uses minZoom) will fail unless the parser is also changed to normalize the key. If you intend to keep the parser behavior, the test should check minzoom instead or be updated alongside a parser change.

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.

1 participant