Skip to content

Commit

Permalink
Merge pull request #1291 from microsoft/develop
Browse files Browse the repository at this point in the history 
Version 2.4.1
  • Loading branch information
@zhangcc01
zhangcc01 authored Oct 22, 2019
2 parents d3e1ce0 + bbeb52e commit 6840b09
Show file tree
Hide file tree
Showing 13 changed files with 158 additions and 79 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# App Center SDK for Android Change Log

## Version 2.4.1

### App Center Distribute

* **[Fix]** Fix a crash and improve logging when downloading an update fails on Android 5+.

___

## Version 2.4.0

### App Center Crashes
Expand Down
50 changes: 37 additions & 13 deletions ...nter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/CrashesAndroidTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,16 @@ public static void setUpClass() {
public void setUp() {
Thread.setDefaultUncaughtExceptionHandler(sDefaultCrashHandler);
SharedPreferencesManager.clear();
for (File logFile : ErrorLogHelper.getErrorStorageDirectory().listFiles()) {
File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles();
assertNotNull(files);
for (File logFile : files) {
if (logFile.isDirectory()) {
for (File dumpDir : logFile.listFiles()) {
for (File dumpFile : dumpDir.listFiles()) {
File[] childFiles = logFile.listFiles();
assertNotNull(childFiles);
for (File dumpDir : childFiles) {
File[] dumpFiles = dumpDir.listFiles();
assertNotNull(dumpFiles);
for (File dumpFile : dumpFiles) {
assertTrue(dumpFile.delete());
}
}
Expand Down Expand Up @@ -141,7 +147,7 @@ private void startFresh(CrashesListener listener) {
AppCenter.setLogLevel(android.util.Log.VERBOSE);
AppCenter.configure(sApplication, "a");

/* Clean logs. */
/* Clean state. */
AppCenter.setEnabled(false);
AppCenter.setEnabled(true).get();

Expand Down Expand Up @@ -343,7 +349,9 @@ public void run() {
thread.start();
thread.join();
verify(uncaughtExceptionHandler).uncaughtException(thread, exception);
assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(2, files.length);
verifyZeroInteractions(crashesListener);

/* Second process: enqueue log but network is down... */
Expand Down Expand Up @@ -374,7 +382,9 @@ public void run() {

/* Waiting user confirmation so no log sent yet. */
verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt());
assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(2, files.length);
verify(crashesListener).shouldProcess(any(ErrorReport.class));
verify(crashesListener).shouldAwaitUserConfirmation();
verifyNoMoreInteractions(crashesListener);
Expand All @@ -386,7 +396,9 @@ public void run() {
verify(mChannel).enqueue(log.capture(), anyString(), eq(CRITICAL));
assertNotNull(log.getValue());
assertEquals(mUserId, log.getValue().getUserId());
assertEquals(1, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(1, files.length);

verify(crashesListener).getErrorAttachments(any(ErrorReport.class));
verifyNoMoreInteractions(crashesListener);
Expand Down Expand Up @@ -420,7 +432,9 @@ public void run() {
});
semaphore.acquire();

assertEquals(0, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(0, files.length);
verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt());
verify(crashesListener).onBeforeSending(any(ErrorReport.class));
verify(crashesListener).onSendingSucceeded(any(ErrorReport.class));
Expand Down Expand Up @@ -459,7 +473,9 @@ public void run() {

/* Waiting user confirmation so no log sent yet. */
verify(mChannel, never()).enqueue(isA(ManagedErrorLog.class), anyString(), anyInt());
assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(2, files.length);
verify(crashesListener).shouldProcess(any(ErrorReport.class));
verify(crashesListener).shouldAwaitUserConfirmation();
verifyNoMoreInteractions(crashesListener);
Expand All @@ -473,7 +489,9 @@ public void run() {
assertEquals(mUserId, managedErrorLog.getValue().getUserId());
assertNotNull(managedErrorLog.getValue().getException());
assertNull(managedErrorLog.getValue().getException().getMinidumpFilePath());
assertEquals(1, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(1, files.length);
verify(crashesListener).getErrorAttachments(any(ErrorReport.class));
verifyNoMoreInteractions(crashesListener);

Expand Down Expand Up @@ -514,12 +532,16 @@ public void run() {
thread.start();
thread.join();
verify(uncaughtExceptionHandler).uncaughtException(thread, exception);
assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(2, files.length);

/* Disable, test waiting for disable to finish. */
Crashes.setEnabled(false).get();
assertFalse(Crashes.isEnabled().get());
assertEquals(0, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(0, files.length);
}

@Test
Expand All @@ -545,6 +567,8 @@ public void run() {
verify(uncaughtExceptionHandler).uncaughtException(thread, exception);

/* Check there are only 2 files: the throwable and the json one. */
assertEquals(2, ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter).length);
File[] files = ErrorLogHelper.getErrorStorageDirectory().listFiles(mMinidumpFilter);
assertNotNull(files);
assertEquals(2, files.length);
}
}
30 changes: 8 additions & 22 deletions ...droidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.junit.Test;

import java.io.File;
import java.lang.reflect.Method;
import java.util.UUID;

import static com.microsoft.appcenter.test.TestUtils.TAG;
Expand Down Expand Up @@ -58,33 +57,20 @@ public void setUp() {
}
}

private void startFresh() throws java.lang.Exception {
private void startFresh() {

/* Configure new instance. */
AppCenterPrivateHelper.unsetInstance();
Crashes.unsetInstance();
AppCenter.setLogLevel(android.util.Log.VERBOSE);
AppCenter.configure(sApplication, "a");

/* Replace channel. */
Method method = AppCenter.class.getDeclaredMethod("getInstance");
method.setAccessible(true);
AppCenter appCenter = (AppCenter) method.invoke(null);
method = AppCenter.class.getDeclaredMethod("setChannel", Channel.class);
method.setAccessible(true);
method.invoke(appCenter, mock(Channel.class));

/*
* Since this is a real Android test, it might actually try to send crash logs
* and will delete files on sending completion. Avoid that by requesting user confirmation.
*/
Crashes.setListener(new AbstractCrashesListener() {

@Override
public boolean shouldAwaitUserConfirmation() {
return false;
}
});
/* Clean state. */
AppCenter.setEnabled(false);
AppCenter.setEnabled(true).get();

/* Replace channel to avoid trying to send logs. */
AppCenter.getInstance().setChannel(mock(Channel.class));

/* Start crashes. */
AppCenter.start(Crashes.class);
Expand All @@ -94,7 +80,7 @@ public boolean shouldAwaitUserConfirmation() {
}

@Test
public void saveWrapperException() throws java.lang.Exception {
public void saveWrapperException() {

class ErrorData {
private String data;
Expand Down
6 changes: 4 additions & 2 deletions ...-distribute/src/main/java/com/microsoft/appcenter/distribute/ReleaseDownloadListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.content.Intent;
import android.net.Uri;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.UiThread;
import android.support.annotation.WorkerThread;
import android.widget.Toast;
Expand Down Expand Up @@ -112,8 +113,9 @@ public boolean onComplete(@NonNull Uri localUri) {

@WorkerThread
@Override
public void onError(@NonNull String errorMessage) {
AppCenterLog.error(LOG_TAG, errorMessage);
public void onError(@Nullable String errorMessage) {
AppCenterLog.error(LOG_TAG, String.format(Locale.ENGLISH, "Failed to download %s (%d) update: %s",
mReleaseDetails.getShortVersion(), mReleaseDetails.getVersion(), errorMessage));
HandlerUtils.runOnUiThread(new Runnable() {

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...stribute/src/main/java/com/microsoft/appcenter/distribute/download/ReleaseDownloader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import android.net.Uri;
import android.support.annotation.AnyThread;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.WorkerThread;

import com.microsoft.appcenter.distribute.ReleaseDetails;
Expand Down Expand Up @@ -83,6 +84,6 @@ interface Listener {
*
* @param errorMessage The message of the exception.
*/
void onError(String errorMessage);
void onError(@Nullable String errorMessage);
}
}
2 changes: 1 addition & 1 deletion ...ava/com/microsoft/appcenter/distribute/download/http/HttpConnectionReleaseDownloader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ synchronized void onDownloadComplete(File targetFile) {
}

@WorkerThread
synchronized void onDownloadError(String errorMessage) {
synchronized void onDownloadError(@Nullable String errorMessage) {
if (isCancelled()) {
return;
}
Expand Down
7 changes: 4 additions & 3 deletions ...n/java/com/microsoft/appcenter/distribute/download/manager/DownloadManagerUpdateTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,19 @@ protected Void doInBackground(Void... params) {
try {
Cursor cursor = downloadManager.query(new DownloadManager.Query().setFilterById(downloadId));
if (cursor == null) {
throw new NoSuchElementException();
throw new NoSuchElementException("Cannot find download with id=" + downloadId);
}
try {
if (!cursor.moveToFirst()) {
throw new NoSuchElementException();
throw new NoSuchElementException("Cannot find download with id=" + downloadId);
}
if (isCancelled()) {
return null;
}
int status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS));
if (status == DownloadManager.STATUS_FAILED) {
throw new IllegalStateException();
int reason = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_REASON));
throw new IllegalStateException("The download has failed with reason code: " + reason);
}
if (status != DownloadManager.STATUS_SUCCESSFUL) {
mDownloader.onDownloadProgress(cursor);
Expand Down
34 changes: 22 additions & 12 deletions ...tribute/src/test/java/com/microsoft/appcenter/distribute/ReleaseDownloadListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public Void answer(InvocationOnMock invocation) {
}

@Test
public void onStartTest() throws Exception {
public void onStart() throws Exception {
ReleaseDetails releaseDetails = mockReleaseDetails(true);
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, releaseDetails);
long mockTime = 1000 * 1000 * 1000;
Expand All @@ -245,7 +245,7 @@ public void onStartTest() throws Exception {
}

@Test
public void dontShowDialogOnZeroProgressTest() throws Exception {
public void doNotShowDialogOnZeroProgress() throws Exception {
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true));

/* Setup progressDialog. */
Expand All @@ -265,7 +265,7 @@ public void dontShowDialogOnZeroProgressTest() throws Exception {
}

@Test
public void dontShowNullDialogOnProgressTest() throws Exception {
public void doNotShowNullDialogOnProgress() throws Exception {

/* If release is mandatory, the progressDialog is not set and equals to null. */
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(false));
Expand All @@ -285,7 +285,7 @@ public void dontShowNullDialogOnProgressTest() throws Exception {
}

@Test
public void dontHideNullDialogTest() throws Exception {
public void doNotHideNullDialog() throws Exception {
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(false));

/* We don't setup progressDialog here by calling showDownloadProgress(). */
Expand All @@ -297,7 +297,7 @@ public void dontHideNullDialogTest() throws Exception {
}

@Test
public void hideDialogTest() throws Exception {
public void hideDialog() throws Exception {
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true));

/* Setup progressDialog. */
Expand All @@ -310,7 +310,7 @@ public void hideDialogTest() throws Exception {
}

@Test
public void showDialogOnProgressTest() throws Exception {
public void showDialogOnProgress() throws Exception {
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true));
when(mProgressDialog.isIndeterminate()).thenReturn(true);

Expand All @@ -329,7 +329,7 @@ public void showDialogOnProgressTest() throws Exception {
}

@Test
public void showIndeterminateDialogOnProgressTest() throws Exception {
public void showIndeterminateDialogOnProgress() throws Exception {
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails(true));
when(mProgressDialog.isIndeterminate()).thenReturn(false);

Expand All @@ -350,17 +350,27 @@ public void showIndeterminateDialogOnProgressTest() throws Exception {
}

@Test
public void onErrorTest() throws Exception {
public void onError() throws Exception {
ReleaseDetails mockReleaseDetails = mockReleaseDetails(true);
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails);
releaseDownloadListener.onError("");
releaseDownloadListener.onError("test error");

/* Verify that completeWorkflow() is called on error. */
verify(mDistribute).completeWorkflow(mockReleaseDetails);
}

@Test
public void onCompleteTest() throws Exception {
public void onErrorNullMessage() throws Exception {
ReleaseDetails mockReleaseDetails = mockReleaseDetails(true);
ReleaseDownloadListener releaseDownloadListener = new ReleaseDownloadListener(mContext, mockReleaseDetails);
releaseDownloadListener.onError(null);

/* Verify that completeWorkflow() is called on error. */
verify(mDistribute).completeWorkflow(mockReleaseDetails);
}

@Test
public void onComplete() throws Exception {
ReleaseDetails mockReleaseDetails = mockReleaseDetails(true);

/* Do not notify the download. */
Expand All @@ -374,7 +384,7 @@ public void onCompleteTest() throws Exception {
}

@Test
public void onCompleteNotifyTest() throws Exception {
public void onCompleteNotify() throws Exception {
boolean mandatoryUpdate = false;
ReleaseDetails mockReleaseDetails = mockReleaseDetails(mandatoryUpdate);

Expand All @@ -389,7 +399,7 @@ public void onCompleteNotifyTest() throws Exception {
}

@Test
public void onCompleteActivityNotResolvedTest() throws Exception {
public void onCompleteActivityNotResolved() throws Exception {
boolean mandatoryUpdate = false;

/* Mock resolving to null activity. */
Expand Down
Loading

0 comments on commit 6840b09

Please sign in to comment.