tests\projection\globe\terrain\fill-planet-pole test is not stable

[HarelM]

I've seen in previous CI runs that fill-planet-pole test is flickering.
@kubapelc how valuable is this test?
From CI runs I see that there is a problem around the pole where the image doesn't look good in all kinds of scenarios, I've also seen this on my dev env using a Mac.

I think the easiest approach would be to remove it, but maybe a different test can provide a similar value for this case...
CC: @birkskyum @ibesora

[kubapelc]

The globe/fill-planet-pole test is stable afaik, globe/terrain/fill-planet-pole (globe+terrain) is the flaky one. The non-terrain test makes sure than fill layers are propertly extrapolated to the poles on globe, and there is an equivalent test for raster layers as well. It looks like the terrain test has a hole in the pole (in the expected image), and it looks like there is some z-fighting or something similar causing instability.

[HarelM]

Yes, sorry, forgot that there is more than one test with the same folder postfix.
The test is failing here:
https://github.com/maplibre/maplibre-gl-js/actions/runs/12293860857/job/34307425714

I've updated the issue's title.

[birkskyum]

The zoom in this test is -0.5, so I can imagine both of these issues can impact the test reliability:

• Globe with/without terrain can zoom too far out #4990
• Mesh is broken near the pole on globe with terrain #4984

Since the test appear to be configured okay, but it's annoying it's flaky, can we temporarily skip it, and note on the terrain pole ticket to re-enable it as part of issue resolution?

The zoom appear very low, and I can imagine it's because it's difficult to control the camera to a point where the pole is visible. If it's the case, then I wonder if this PR makes it easier:

• Avoid constraining with Mercator transform when Globe is used #5186

[HarelM]

It can exist in the git history, we can create an issue and add the style.json file there, or we can keep this issue open and remove the test for now until we decide to solve this issue.
But I have removed ignored tests in the past as it is a bad practice, so I would like to avoid adding skipped tests...

[birkskyum]

Okay, we can do that. I'm not trying to be argumentative at all, or be the judge what's bad practice. I genuinely find it difficult to figure what's good practice for the main branch in this CI/CD pipeline era. The formal training I've had in the area of code testing was dominated by recommendations of following a TDD doctrine, and in that mindset it seems completely opposite to delete a failing test solely for the reason that correctly flagging a bug. With TDD I'd always have this list of failing tests ahead of me until the project was done. Now we have a situation where that might still be possible to some extend on a feature branch, but the main branch has to be kept CI-green at all times, otherwise it'll block all incoming PRs, pre-releases etc.

If it was possible, I'd probably prefer to have the CI on main branch run all the test (so none are skipped/ignored), but allow a certain tests that hit bugs or lack implementation to fail "silently" so it's showing up, but it doesn't stop the CI run.

[HarelM]

TDD means you are writing tests first and then making them pass.
Keeping a failing tests only gets you to ignore the CI results as you are saying that "this is probably just another flaky test".
You are right that the best practice is to fix the test, but I'm not sure we can do that ATM and it's best not to keep flaky tests.
We can open a branch, add that test there and return to that branch when we have the time to fix that, but I'm not sure this would work either.
So I prefer not flaky tests over completeness of coverage I guess is what I'm saying.