mapbox / mapbox-gl-test-suite

Rendering tests for Mapbox GL
6 stars 12 forks source link

regression test for mapbox-gl-native#7241 #188

Closed ivovandongen closed 7 years ago

ivovandongen commented 7 years ago

Fixes #187

ivovandongen commented 7 years ago

@jfirebaugh Strangely enough this test case also succeeds against master although this is exactly the case that https://github.com/mapbox/mapbox-gl-native/issues/7241 fixes.

jfirebaugh commented 7 years ago

@ivovandongen It looks like the unit test in #7242 also passes without the fix present. In this case, I'm seeing Style::onLayerVisibilityChanged() trigger both Update::RecalculateStyle and Update::Layout. The former results in enabled getting set to true, and then the tiles get loaded via style->updateTiles(parameters).

Is there some additional setup needed to trigger the issue that these tests are missing?

ivovandongen commented 7 years ago

@jfirebaugh I've been looking into this. I've fixed the unit test, so it fails on master in the map test. It turns out there is a specific sequence that needs to happen so that the cache is filled with invalid tiles.

To reproduce this in a render test I need to be able to call render after certain steps so that the tiles are pruned and put into cache. However, I can't seem to do that right now as the render function takes a callback function as a second argument:

TypeError: Second argument must be a callback function
jfirebaugh commented 7 years ago

@ivovandongen So the sequence is:

?

You should be able to simulate that same sequence in a render test using the wait command, which has the same effect as test::render in unit tests.

ivovandongen commented 7 years ago

@jfirebaugh That seems to be exactly the test I've implemented in https://github.com/mapbox/mapbox-gl-test-suite/pull/188/commits/989fb6d1a148930aa66409a96671dfce6e3e1535. However, it doesn't seem to render on wait operations though (verified with some log statements).

I see in https://github.com/mapbox/mapbox-gl-native/blob/424c05be5e3fbd45e52d8e3a6cf048cbd5703b8d/platform/node/test/suite_implementation.js#L68 that there is a render call in wait. However, after a setVisiblity operation, the loaded state of the map/style/source doesn't change and render is never called, instead the next operation is performed immediately.

jfirebaugh commented 7 years ago

Looks like there are a couple of issues:

I'll push a fix for these issues to the branch.