Debug git history on test-render

[wipfli]

test-render currently fails for many tests. I would like to see if in the past this was not the case. GitHub makes it easy to change node versions, which is used in the test-render.yml workflow to go back to version 10 of node.

My focus is on debug/overdraw because it is the first test that fails.

[wipfli]

Great, so debug/overdraw passed before typescript and fails now. See https://github.com/maplibre/maplibre-gl-js/pull/365/checks?check_run_id=3644859539.

[wipfli]

Before the typescript migration only one render test out of 1200 fails: debug/tile-overscaled. See https://github.com/maplibre/maplibre-gl-js/runs/3644885743?check_suite_focus=true.

[HarelM]

Failing tests due to usage of mapbox:// protocol is expected as latest version does not have support for mapbox:// protocol (I removed it).
I'll share the rest of my thought in #326

[wipfli]

To give an example, debug/overdraw should look like this (expected.png):

However, with the latest version it looks like this (actual.png):

[wipfli]

Failing tests due to usage of mapbox:// protocol is expected

Yes, but only one render test uses it. See #326 (comment).

[HarelM]

I see your point. Interesting... I saw a report with a lot more failing tests, not sure where I saw it (I think it was #343)...
In any case, we probably need to go over the typescript branch changes and try to find out which changes are causing the test to fail, the problem is that doing it in github is super painful as most of the files are kept closed and there's no good way to open them... :-(
I'm also not sure how to easily find the root cause for failing tests, but we'll need to learn, I guess :-)
I also saw some artifact lines when using safari, which might explain the failing tests. We should take care of this before releasing 2.0 official version.

[wipfli]

Here is a way to see all changes between just before migrating to typescript, and where we are now (Split CI):

49bc4ca...37bcaf5

[HarelM]

I know, but you can't search there and you need to click on "show diff" like 400 times to see all the file changes content... :-(

[wipfli]

To load all diffs go to the Files changed tab, open the console, and run:

for(i=0; i<document.getElementsByClassName('js-button-text').length; i++){ document.getElementsByClassName('js-button-text')[i].click();}

Taken from http://www.antleon.com/2019/12/github-expand-all-collapsed-file-in-pr-diffs/

[HarelM]

Nice!! Thanks!

[wipfli]

The render test debug/tile-overscale is the only one that fails just before the migration to typescript. I wonder if this test always failed or if we introduced some changes that made it fail. For this I now look at debug/tile-overscale at the time we first released this library (tag v1.14.0).

Workflow result is here. This test already failed at tag v1.14.0.

Let's check now if this test passed before we forked, i.e. at f87090b. OK, it failed already before we forked, see here.

[msbarry]

I tried git-bisect on commits in https://github.com/maplibre/maplibre-gl-js/pull/209/commits on my 2015 macbook pro - it wasn't too helpful since there were large ranges of commits where the render tests weren't runnable, but one thing I found was that if you check out a963b6e (about halfway through) then git checkout main tests && npm ci && npm run build-dev && npm run test-render then only 13 render tests were failing at that point:

* failed text-field/formatted-images-vertical
* failed text-writing-mode/point_label/cjk-arabic-vertical-mode
* failed text-writing-mode/point_label/cjk-horizontal-vertical-mode
* failed text-writing-mode/point_label/cjk-multiline-vertical-horizontal-mode
* failed text-writing-mode/point_label/cjk-punctuation-vertical-mode
* failed text-writing-mode/point_label/cjk-variable-anchors-vertical-horizontal-mode-icon-text-fit
* failed text-writing-mode/point_label/cjk-variable-anchors-vertical-horizontal-mode
* failed text-writing-mode/point_label/cjk-variable-anchors-vertical-mode
* failed text-writing-mode/point_label/cjk-vertical-horizontal-mode
* failed text-writing-mode/point_label/cjk-vertical-mode
* failed text-writing-mode/point_label/mixed-multiline-vertical-horizontal-mode-icon-text-fit
* failed text-writing-mode/point_label/mixed-multiline-vertical-horizontal-mode
* errored video/default

(Also, debug/tile-overscaled seems to pass for me there, and before the typescript conversion...? Might be some flakiness/inconsistency in CI on that one?)

So the text writing mode test failure was introduced somewhere between cf52cc9...a963b6e and the rest between a963b6e...8d789b7

Looking at the failing tests, it definitely seems like we want to fix them before a 2.0 release - sorry I didn't get these render tests up in CI early on, that would have made it easier to fix these during the conversion 😞

[HarelM]

The branch was not compiling for some time so this is hard to figure out. If the CI was up and running at that time it would only prevent the merge to main, figuring out what causes the tests to fail would have still been hard, much like now...
You can use the test-render-revert branch to try and see what changes causes the tests to pass. I think I did the obvious reverts, we're probably left with the not so obvious...
Thanks for looking into it! Much appreciated. I need a set of new eyes on this. :-)

[wipfli]

I am running the tests now on commit a963b6e which is one of the steps of the typescript migration. Thanks @msbarry for the bisection of the typescript branch.

[wipfli]

In CI on macos, the mentioned commit is failing. Any idea what is different from your environment @msbarry?

[wipfli]

      matrix:
        ref:
          - f87090b70b8a6fbde1a779c44933ce32bffc5d2c # before fork
          - 49bc4ca45aba6b6af98df7e1e91735dcb0d6f20f # before typescript