Node render test suite on Android #12041

tobrun · 2018-05-31T15:57:06Z

Continuation of #10045, Closes #10047.

This PR enables running the node render test suite on Android:

Example run can be seen here, executing these tests can be done with:

make run-android-render-test-arm-v7

When running the render tests the following happens

copies test files from mapbox-gl-js/test/integration/* to app assets folder
builds application and executes RenderTest.java instrumentation test
pulls images from device and compares results with PixelMatch

The following items are open, thinking about tackling them in separate PRs:

update $(BUILD_DEPS)
use ignore.json from node to ignore tests
integrate in CI (nightly integration)
ticket out
- ignored tests that have operations
- integrating expressions tests
- integrating query rendered features tests
- ticket out failing render tests

ChrisLoer · 2018-05-31T19:56:47Z

It looks like it would be relatively straightforward to extend this approach to the query tests and the expression tests? Being able to run expression tests on Android would be useful for the Collator expression which has an Android-specific implementation (#11869).

tobrun · 2018-06-01T08:12:44Z

Great idea @ChrisLoer, quickly glanced at tests in mapbox-gl-js/test/integration/query-test/..:

These tests require some kind of integration of QueryRenderFeatures functionality (see queryGeometry in json below). This kind of feature isn't implemented on MapSnapshotter but we can definitely add this.

{
  "version": 8,
  "metadata": {
    "test": {
      "height": 256,
      "queryGeometry": [
        10,
        100
      ]
    }
  },
  "center": [
    13.418056,
    52.499167
  ],
  "zoom": 14,
  "sources": {
    "mapbox": {
      "type": "vector",
      "maxzoom": 14,
      "tiles": [
        "local://tiles/{z}-{x}-{y}.mvt"
      ]
    }
  },
  "layers": [
    {
      "id": "road",
      "type": "circle",
      "source": "mapbox",
      "source-layer": "road",
      "paint": {
        "circle-radius": 10
      },
      "interactive": true
    }
  ]
}

That said, this fits inline with some other render tests that I had to ignore for a similar reason (I'm ignoring all tests with test{operations{ as the mapsnapshotter doesn't have the ability to change the map transformation (eg. change zoom level and wait for that to take effect).

I will be ticketing these improvements out with the rest of the todo items, thank you for 👀 'ing this!

LukasPaczos

This look amazing! 🚀
A couple of comments below.

LukasPaczos · 2018-06-01T09:43:54Z

...orm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/LocalRequestTask.java

+    int size = input.available();
+    byte[] buffer = new byte[size];
+    input.read(buffer);
+    input.close();


What about using try-with-resources?

LukasPaczos · 2018-06-01T09:44:47Z

...GLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/render/RenderTest.java

+    setupIdlingResource();
+  }
+
+  private void grantWriteRuntimePermission() {


This can be achieved with handy JUnit Rules.

LukasPaczos · 2018-06-01T09:45:57Z

...GLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/render/RenderTest.java

+
+  @Before
+  public void beforeTest() {
+    IdlingPolicies.setMasterPolicyTimeout(30, TimeUnit.MINUTES);


Extract the timeout to a variable?

LukasPaczos · 2018-06-01T09:46:08Z

...GLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/render/RenderTest.java

+  private void setupIdlingResource() {
+    try {
+      Timber.e("@Before test: register idle resource");
+      IdlingPolicies.setIdlingResourceTimeout(30, TimeUnit.MINUTES);


Same as above.

LukasPaczos · 2018-06-01T09:47:40Z

...DKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/render/RenderTestActivity.java

+    }
+
+    private static String loadStyleJson(AssetManager assets, String category, String test) throws IOException {
+      InputStream input = assets.open(String.format("%s/%s/%s/style.json", RENDER_TEST_BASE_PATH, category, test));


try-with-resources would be nice, also extracting whole path format or at least the file name to a variable.

LukasPaczos · 2018-06-01T10:04:48Z

...DKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/render/RenderTestActivity.java

+      FileOutputStream out = new FileOutputStream(filePath);
+      testResult.compress(Bitmap.CompressFormat.PNG, 100, out);
+      out.flush();
+      out.close();


Suggesting try-with-resources again.

LukasPaczos · 2018-06-01T10:06:29Z

...DKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/render/RenderTestActivity.java

+    }
+
+    private static File createTestResultRootFolder() {
+      File testResultDir = new File(Environment.getExternalStorageDirectory() + File.separator + "mapbox");


What about adding "/render" to the root path?

LukasPaczos · 2018-06-01T10:08:33Z

...DKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/render/RenderTestActivity.java

+  //
+  // Callback configuration to notify test executor of test finishing
+  //
+  public interface OnSnapshotReadyListener {


This interface's name and method's name are a bit ambiguous. Could we rename it to something along the lines of OnTestSnapshotsReadyListener?

LukasPaczos · 2018-06-01T10:53:21Z

...GLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/render/RenderTest.java

+    try {
+      Timber.e("@Before test: register idle resource");
+      IdlingPolicies.setIdlingResourceTimeout(30, TimeUnit.MINUTES);
+      Espresso.registerIdlingResources(idlingResource = new SnapshotterIdlingResource(rule.getActivity()));


This method is deprecated, could you use IdlingRegistry.register(IdlingResource...) instead?

LukasPaczos · 2018-06-01T10:57:43Z

...GLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/render/RenderTest.java

+  @After
+  public void afterTest() {
+    Timber.e("@After test: unregister idle resource");
+    Espresso.unregisterIdlingResources(idlingResource);


Same as above.

LukasPaczos · 2018-06-01T12:26:19Z

...orm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/LocalRequestTask.java

+      int size = input.available();
+      buffer = new byte[size];
+      input.read(buffer);
+      input.close();


No need to explicitly close the resource here.

LukasPaczos · 2018-06-01T12:27:04Z

platform/android/MapboxGLAndroidSDKTestApp/src/main/AndroidManifest.xml

@@ -791,6 +791,18 @@
        <activity
            android:name=".activity.style.FillExtrusionStyleTestActivity"
            android:screenOrientation="portrait" />
+        <activity


This was only the debug setup. Can we remove it?

tobrun added the Android Mapbox Maps SDK for Android label May 31, 2018

tobrun self-assigned this May 31, 2018

tobrun force-pushed the tvn-render-test branch from 86b997d to 8580bd7 Compare May 31, 2018 16:01

This was referenced Jun 1, 2018

Integration test: query-test #12044

Closed

Integration test: expression-test #12045

Closed

tobrun force-pushed the tvn-render-test branch from 8580bd7 to 93225f8 Compare June 1, 2018 09:29

tobrun requested a review from LukasPaczos June 1, 2018 09:33

tobrun force-pushed the tvn-render-test branch 2 times, most recently from 61b6115 to 97b56de Compare June 1, 2018 09:41

LukasPaczos suggested changes Jun 1, 2018

View reviewed changes

LukasPaczos reviewed Jun 1, 2018

View reviewed changes

tobrun force-pushed the tvn-render-test branch 4 times, most recently from dcfa87d to 9810d8c Compare June 1, 2018 12:14

LukasPaczos reviewed Jun 1, 2018

View reviewed changes

tobrun force-pushed the tvn-render-test branch from 9810d8c to 09751b2 Compare June 1, 2018 12:34

LukasPaczos approved these changes Jun 1, 2018

View reviewed changes

tobrun force-pushed the tvn-render-test branch 4 times, most recently from bdf6b5d to 3f04f13 Compare June 1, 2018 16:49

[android] - Render tests with PixelMatch

94027ae

tobrun force-pushed the tvn-render-test branch from 3f04f13 to 94027ae Compare June 1, 2018 17:49

tobrun merged commit be66de6 into master Jun 1, 2018

tobrun deleted the tvn-render-test branch June 1, 2018 18:06

tobrun mentioned this pull request Jun 5, 2018

Integration test: improve render-test output #12062

Closed

freszu mentioned this pull request Jun 7, 2018

Revert support for java 1.7 or figure a way to use java 8 features while maintaining compatibility for Android before N mapbox/mapbox-java#832

Closed

tobrun mentioned this pull request Jun 8, 2018

Render test integration: intermediate results #12095

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node render test suite on Android #12041

Node render test suite on Android #12041

tobrun commented May 31, 2018 •

edited

Loading

ChrisLoer commented May 31, 2018

tobrun commented Jun 1, 2018

LukasPaczos left a comment

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

LukasPaczos Jun 1, 2018

Node render test suite on Android #12041

Node render test suite on Android #12041

Conversation

tobrun commented May 31, 2018 • edited Loading

ChrisLoer commented May 31, 2018

tobrun commented Jun 1, 2018

LukasPaczos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobrun commented May 31, 2018 •

edited

Loading