test: add antimeridian splitting tests for low npoints values#76
test: add antimeridian splitting tests for low npoints values#76thomas-hervey wants to merge 1 commit intospringmeyer:chore/esmfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Jest/TypeScript regression tests to cover antimeridian splitting behavior in GreatCircle.Arc() for several Pacific routes, with special focus on low npoints values implicated in issue #75.
Changes:
- Add a new
test/antimeridian.test.tssuite covering three antimeridian-crossing routes atnpoints=100andnpoints=10. - Add a non-crossing route assertion (Seattle → DC) to confirm non-dateline behavior remains stable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('with npoints=100', () => { | ||
| for (const { name, start, end } of PACIFIC_ROUTES) { | ||
| test(`${name} produces a split MultiLineString`, () => { | ||
| const result = new GreatCircle(start, end).Arc(100, { offset: 10 }).json(); | ||
| expect(result.geometry.type).toBe('MultiLineString'); | ||
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | ||
| }); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
This file duplicates the existing Tokyo/Auckland/Shanghai antimeridian split assertions already present in test/great-circle.test.ts for npoints=100. Consider keeping the new low-npoints coverage but avoid repeating the npoints=100 suite (or move these cases into the existing "Dateline crossing" block) to reduce duplicated fixtures and maintenance overhead.
| describe('with npoints=100', () => { | |
| for (const { name, start, end } of PACIFIC_ROUTES) { | |
| test(`${name} produces a split MultiLineString`, () => { | |
| const result = new GreatCircle(start, end).Arc(100, { offset: 10 }).json(); | |
| expect(result.geometry.type).toBe('MultiLineString'); | |
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | |
| }); | |
| } | |
| }); |
| test(`${name} splits correctly`, () => { | ||
| const result = new GreatCircle(start, end).Arc(10, { offset: 10 }).json(); | ||
| expect(result.geometry.type).toBe('MultiLineString'); | ||
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | ||
| }); |
There was a problem hiding this comment.
As written, the npoints=10 tests will fail on the current GreatCircle.Arc implementation (it often returns an unsplit LineString for Tokyo→LAX and Shanghai→SFO because the split detector only triggers for longitude diffs > 360 - offset). If this PR is intended to be tests-only, mark the known-bad cases with test.failing/test.todo (or skip with a link to #75) to keep CI green; otherwise include the corresponding implementation fix in this PR.
| test(`${name} splits correctly`, () => { | |
| const result = new GreatCircle(start, end).Arc(10, { offset: 10 }).json(); | |
| expect(result.geometry.type).toBe('MultiLineString'); | |
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | |
| }); | |
| if (name === 'Tokyo → LAX' || name === 'Shanghai → SFO') { | |
| test.skip(`${name} splits correctly (known issue, see #75)`, () => { | |
| const result = new GreatCircle(start, end).Arc(10, { offset: 10 }).json(); | |
| expect(result.geometry.type).toBe('MultiLineString'); | |
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | |
| }); | |
| } else { | |
| test(`${name} splits correctly`, () => { | |
| const result = new GreatCircle(start, end).Arc(10, { offset: 10 }).json(); | |
| expect(result.geometry.type).toBe('MultiLineString'); | |
| assertSplitAtAntimeridian((result.geometry as MultiLineString).coordinates); | |
| }); | |
| } |
| function assertSplitAtAntimeridian(coords: number[][][]) { | ||
| const seg0 = coords[0]; | ||
| const seg1 = coords[1]; | ||
|
|
||
| expect(seg0).toBeDefined(); | ||
| expect(seg1).toBeDefined(); | ||
|
|
||
| if (!seg0 || !seg1) return; // narrow for TS | ||
|
|
||
| const lastOfFirst = seg0[seg0.length - 1]; | ||
| const firstOfSecond = seg1[0]; | ||
|
|
||
| expect(lastOfFirst).toBeDefined(); | ||
| expect(firstOfSecond).toBeDefined(); | ||
|
|
||
| if (!lastOfFirst || !firstOfSecond) return; // narrow for TS | ||
|
|
||
| // Both sides of the split must be at ±180 | ||
| expect(Math.abs(lastOfFirst[0] ?? NaN)).toBeCloseTo(180, 1); | ||
| expect(Math.abs(firstOfSecond[0] ?? NaN)).toBeCloseTo(180, 1); | ||
|
|
||
| // Latitudes must match — no gap at the antimeridian | ||
| expect(lastOfFirst[1] ?? NaN).toBeCloseTo(firstOfSecond[1] ?? NaN, 3); | ||
| } |
There was a problem hiding this comment.
assertSplitAtAntimeridian only checks abs(lon) ≈ 180, so it would still pass if both segment endpoints use the same sign (e.g. 180/180) or if the geometry contains >2 segments with later bad splits. To prevent false positives, assert coords.length === 2, both segments are non-empty, and that the split longitudes are exactly 180 and -180 (opposite signs) for the adjacent endpoints (matching the implementation and existing tests in test/great-circle.test.ts).
|
Those copilot reviews ^ were not supposed to be automatic, but I will look through them and address as needed. |
Branch to addressing partial regression #75. While testing
v1.0.0-beta.1in @turf/great-circle, I found that antimeridian splitting fails for some Pacific routes when npoints is low.Related: #69, #71
This first commit adds tests covering three antimeridian-crossing routes (Tokyo→LAX, Auckland→LAX, Shanghai→SFO) at both npoints=100 and npoints=10. All three pass at 100 points. At 10 points, Tokyo→LAX and Shanghai→SFO return an unsplit LineString instead of a MultiLineString.