Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Espresso tests rework #13576

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Espresso tests rework #13576

merged 1 commit into from
Jan 10, 2019

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Dec 13, 2018

This PR does a cleanup of our instrumentation tests by making a distinction between tests that:

  • load a style through the hosting activity (eg. activity sanity tests)
  • load a style through the test (eg. testing business logic on top of style)

Notable changes:

  • moved activity based tests to instrumented unit tests if possible
  • removed the SingleActivity/SingleFragment setup migrated with LocationComponent
  • performs cleanup of unused methods/classes
  • remove ignored tests that have been ignored for a while

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Dec 13, 2018
@tobrun tobrun self-assigned this Dec 13, 2018
@tobrun tobrun force-pushed the tvn-espresso-test branch 3 times, most recently from 34ff603 to 7133ace Compare December 14, 2018 08:42
Copy link
Member

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

Minor request, otherwise, looks good 🚀

@@ -27,7 +27,7 @@ public class <%- activity %>Test extends BaseActivityTest {

@Test
public void testSanity() {
validateTestSetup();
// no-op
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this test all-together?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now I would keep the sanity tests around, updated above block however with assertions for mapboxMap, style and fully loaded.

public void initMap(MapboxMap mapboxMap) {
super.initMap(mapboxMap);
mapboxMap.getStyle(style -> {
if (resourceCallback != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add an assertion checking that the style is fully loaded?

public void initMap(MapboxMap mapboxMap) {
super.initMap(mapboxMap);
mapboxMap.setStyle("asset://streets.json", style -> {
this.style = style;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* Uses {@link LoadStyleIdlingResource} to load "assets/streets.json" as style.
* </p>
*/
public class EspressoActivityTest extends BaseActivityTest {
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename EspressoActivityTest to EspressoTest? Similarly with BaseActivityTest. It's a little bit hard to read and distinguish between EspressoTestActivity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants