Skip to content

test: add antimeridian splitting tests for low npoints values#76

Draft
thomas-hervey wants to merge 1 commit intospringmeyer:chore/esmfrom
thomas-hervey:test/antimeridian-low-npoints
Draft

test: add antimeridian splitting tests for low npoints values#76
thomas-hervey wants to merge 1 commit intospringmeyer:chore/esmfrom
thomas-hervey:test/antimeridian-low-npoints

Conversation

@thomas-hervey
Copy link
Copy Markdown
Contributor

Branch to addressing partial regression #75. While testing v1.0.0-beta.1 in @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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.ts suite covering three antimeridian-crossing routes at npoints=100 and npoints=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.

Comment on lines +37 to +46
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);
});
}
});

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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);
});
}
});

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +53
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);
});
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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);
});
}

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +34
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);
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
@thomas-hervey
Copy link
Copy Markdown
Contributor Author

Those copilot reviews ^ were not supposed to be automatic, but I will look through them and address as needed.

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.

3 participants