test(js) refactor decoding tests #738

Turtelll · 2025-12-03T13:25:12Z

This PR refactors the decoder test suite to improve test quality by introducing shared test utilities and rewriting tests to use proper data encoding.
Before tests used raw byte streams or mocked metadata objects. Now tests properly encode data using encoding utilities.

… compare the results

…into refactor-tests merge recent changes

…gTestUtils.ts

…into refactor-tests

codecov-commenter · 2025-12-03T13:27:59Z

Codecov Report

❌ Patch coverage is 98.62385% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.93%. Comparing base (e218598) to head (134cd3f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
ts/src/decoding/decodingTestUtils.ts	97.84%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #738      +/-   ##
==========================================
+ Coverage   83.58%   87.93%   +4.34%     
==========================================
  Files          53       54       +1     
  Lines        2345     2552     +207     
  Branches      537      572      +35     
==========================================
+ Hits         1960     2244     +284     
+ Misses        357      286      -71     
+ Partials       28       22       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

HarelM · 2025-12-03T14:57:48Z

ts/src/decoding/decodingTestUtils.ts

I might be reading this wrong, but I see some encoding methods that are not "hard coded".
If this is the case, and there might be in the far future a javascript encoder, we might as well place these methods in files that are symmetrical to the decoding files (in terms of naming).
Later on, we could use them in the test like you did.
Let me know if this makes sense.

It seems there's a lot of logic in this file, I'm not sure if it is related to the previous comment or not, but it's worth taking it into account.

I mean i can place them in a encoder file structure, but the encodings I included for the tests are really only the most basic encodings and half of the logic in this file is needed for creating stream(metadata). If I were to set those up manually we would have at least 10000 of extra code. The only thing that is "hard coded" are the FSST Symbol Table Streams.

TLDR: half of decodingTestUtils.ts is only for test setup and encodings like ZigZag can be moved to a symmetrical encoder directory. But I am not sure if this makes sense right now

Let's start with something, move as much code as possible to encoding folder/files.
If you can take a bit more time to create basic encoding methods that can facilitate for "full cycle encode-decode" tests that would be the best approach, but I don't know how complicated that is, I would assume that for the basic stuff this should be trivial.
This will both make the tests easier to read and maintain and help us in the future if we decide to create a js encoder.

Ok i will move them to encoding folders and add tests where coverage is still lacking.

"full cycle encode-decode" tests are already happening

HarelM · 2025-12-03T15:12:12Z

Please checkout the coverage of the decoding files:

propertyDecoder.ts
integerStreamDecoder.ts
integerDecodingUtils.ts

I would expect those to only have missing coverage for some edge cases when throwing, or code that we don't have a good way to test (morton?).
Although, if we can't reach that code, it might be dead code that needs to be removed.
But I'm guessing a method like decodeDoubleColumn should be covered in tests, for example.

…ropertyDecoder.spec.ts

for more information, see https://pre-commit.ci

…-tile-spec into refactor-tests

HarelM · 2025-12-04T13:51:38Z

ts/src/decoding/decodingTestUtils.ts

+ * - Varints: numValues, byteLength
+ * - If RLE: Varints: runs, numRleValues
+ */
+export function encodeStreamMetadata(metadata: StreamMetadata | RleEncodedStreamMetadata): Uint8Array {


Shouldn't this be part of the encoding?

It is really specific to the testdata streams so i would keep it here

HarelM · 2025-12-04T13:54:40Z

ts/src/decoding/integerStreamDecoder.spec.ts

+    it("should decode NONE signed", () => {
+        const metadata = createStreamMetadata(LogicalLevelTechnique.NONE);
+        // ZigZag encoded: positive values are multiplied by 2
+        const values = new Float64Array([4, 10, 6]); // zigzag encoded [2, 5, 3]


is it possible to create something like encodeFloat64Buffer([2,5,3]) to improve full-circle readability?

HarelM · 2025-12-04T13:57:09Z

ts/src/encoding/encodingUtils.ts

I would expect this file to have similar folder structure to encoding, this way there's a way to see which method is the counter part of the decoding method.
I would expect to heve integerStremEncoder file, propertyEncoder file etc.

I understand what you mean but that would require a whole encoder for all the different streams and at that point we would have half of a fully functioning MLT encoder. So basically it would take a lot more effort then just creating streams like I do in the test utils. But the tests would be verry easy to adapt to using a fully implemented encoder once there is one.

I think there might be a middle ground here, improve test readability by adding some encoders without implementing everything...

Yes but I think the tests are testing the decoder well enough and full cycle testing could wait until we introduce a encoder. There are still other gaps like fastPfor decoding we are missing that should be fixed before we spend a lot of time on this.

The tests here seems very low level decoding - int, double, float etc, I'm failing to see how this would be a big effort...

Benedikt Vogl added 9 commits December 1, 2025 15:51

refactor integerStreamDecoder tests to use encoded data to docode and…

a4ed712

… compare the results

Merge branch 'main' of https://github.com/maplibre/maplibre-tile-spec …

c02f57e

…into refactor-tests merge recent changes

refactor propertyDecoder.spec.ts and move hleper functions to decodin…

df7cdea

…gTestUtils.ts

refactor stringDecoder.spec.ts

a7194d1

removed unused util

6571ed6

add tests for decodeSharedDictionary

15421b6

run formating

34e9b3c

run formating

1bd06ce

Merge branch 'main' of https://github.com/maplibre/maplibre-tile-spec …

652da10

…into refactor-tests

Turtelll requested a review from HarelM December 3, 2025 13:25

Turtelll self-assigned this Dec 3, 2025

Turtelll changed the title ~~Refactor tests~~ test(js) refactor decoding tests Dec 3, 2025

Turtelll added the javascript Pull requests that update Javascript code label Dec 3, 2025

Turtelll marked this pull request as ready for review December 3, 2025 13:44

HarelM reviewed Dec 3, 2025

View reviewed changes

Benedikt Vogl and others added 13 commits December 4, 2025 10:07

add tests for double property Columns

985b487

add tests for unsinged int property Columns and column skipping

7f143bd

add tests for unsinged int property Columns and column skipping

20098b4

add tests for string propertyColumn and skipping complex columns in p…

96ec2d2

…ropertyDecoder.spec.ts

move stream/column creators to util file

496d468

fix string propertyColumn test

11873e2

add tests for decodeIntStream to integerStreamDecoder.spec.ts

3760065

add tests for decodeFloat64Buffer to integerStreamDecoder.spec.ts

ecea015

add tests for decodeNullableIntStream to integerStreamDecoder.spec.ts

c68a637

add tests for decodeIntStream with morton and a delta encoder

cdee902

move encodings to new folder

315fb35

[pre-commit.ci] auto fixes from pre-commit.com hooks

3155e85

for more information, see https://pre-commit.ci

clean up decodingTestUtils.ts

8e50b85

Benedikt Vogl added 2 commits December 4, 2025 13:59

Merge branch 'refactor-tests' of https://github.com/Turtelll/maplibre…

c3613d5

…-tile-spec into refactor-tests

clean up encodingUtils.ts

134cd3f

Turtelll requested a review from HarelM December 4, 2025 13:07

HarelM reviewed Dec 4, 2025

View reviewed changes

test(js) refactor decoding tests #738

Are you sure you want to change the base?

test(js) refactor decoding tests #738

Conversation

Turtelll commented Dec 3, 2025

Uh oh!

codecov-commenter commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarelM commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Turtelll Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Dec 3, 2025 •

edited

Loading

Turtelll Dec 4, 2025 •

edited

Loading