Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve how in progress tests are displayed #673

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.openobservatory.ooniprobe.item.*;
import org.openobservatory.ooniprobe.model.database.Network;
import org.openobservatory.ooniprobe.model.database.Result;
import org.openobservatory.ooniprobe.model.database.ResultExtensions;
import org.openobservatory.ooniprobe.model.database.Result_Table;
import org.openobservatory.ooniprobe.test.suite.*;

Expand Down Expand Up @@ -162,9 +163,11 @@ void queryList() {
for (DatedResults group : list) {
items.add(new DateItem(group.getGroupedDate()));
for (Result result : group.getResultsList()) {
if (result.countTotalMeasurements() == 0)
if (ResultExtensions.getStatus(result) == -1){
items.add(new ProgressItem(result, this, this));
} else if (result.countTotalMeasurements() == 0) {
items.add(new FailedItem(result, this, this));
else {
} else {
switch (result.test_group_name) {
case WebsitesSuite.NAME:
items.add(new WebsiteItem(result, this, this));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
import org.openobservatory.ooniprobe.databinding.ItemFailedBinding;
import org.openobservatory.ooniprobe.model.database.Result;

import java.util.Date;
import java.util.Locale;
import static java.util.concurrent.TimeUnit.*;

import localhost.toolkit.widget.recyclerview.HeterogeneousRecyclerItem;

Expand All @@ -41,24 +39,13 @@ public FailedItem(Result extra, View.OnClickListener onClickListener, View.OnLon
viewHolder.binding.icon.setImageResource(extra.getTestSuite().getIcon());
viewHolder.binding.testName.setText(extra.getTestSuite().getTitle());
String failure_msg = viewHolder.itemView.getContext().getString(R.string.TestResults_Overview_Error);
if (extra.failure_msg != null) {
if (extra.failure_msg != null)
failure_msg += " - " + extra.failure_msg;
} else {
// NOTE: If the test is running for more than 5 minutes, we assume it's stuck or failed,
// and we show the default error message.
long MAX_DURATION = MILLISECONDS.convert(5, MINUTES);
long duration = new Date().getTime() - extra.start_time.getTime();
if (duration < MAX_DURATION) {
failure_msg = viewHolder.itemView.getContext()
.getString(R.string.Dashboard_Running_Running)
.replace(":","");
}
}
viewHolder.binding.subtitle.setText(failure_msg);
viewHolder.binding.startTime.setText(DateFormat.format(DateFormat.getBestDateTimePattern(Locale.getDefault(), "yMdHm"), extra.start_time));
}

class ViewHolder extends RecyclerView.ViewHolder {
public class ViewHolder extends RecyclerView.ViewHolder {
ItemFailedBinding binding;

ViewHolder(ItemFailedBinding binding) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.openobservatory.ooniprobe.item

import android.view.View
import org.openobservatory.ooniprobe.R
import org.openobservatory.ooniprobe.model.database.Result

class ProgressItem(
result: Result,
onClickListener: View.OnClickListener,
onLongClickListener: View.OnLongClickListener
) : FailedItem(result, onClickListener, onLongClickListener) {
override fun onBindViewHolder(viewHolder: ViewHolder) {
super.onBindViewHolder(viewHolder)
viewHolder.binding.subtitle.text = viewHolder.itemView.context
.getString(R.string.Dashboard_Running_Running)
.replace(":", "");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.openobservatory.ooniprobe.model.database

import com.raizlabs.android.dbflow.config.FlowManager
import com.raizlabs.android.dbflow.structure.database.FlowCursor
import org.openobservatory.ooniprobe.common.AppDatabase


class ResultExtensions {

companion object {

@JvmStatic
fun Result.getStatus(): Int {
val queryById = """
SELECT
CASE
WHEN COUNT(m.result_id) = 0 THEN -1
ELSE SUM(CASE WHEN m.is_done = 0 THEN 1 ELSE 0 END)
END AS in_progress_count
FROM
Result r
LEFT JOIN
Measurement m ON r.id = m.result_id
WHERE
r.id = ?
""".trimIndent()
val cursor: FlowCursor = FlowManager.getDatabase(AppDatabase::class.java)
.writableDatabase
.rawQuery(queryById, arrayOf(id.toString()))

var inProgressCount = 0
if (cursor.moveToFirst()) {
cursor.getColumnIndex("in_progress_count").let {
if (it == -1) {
return@let
Copy link
Member

Choose a reason for hiding this comment

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

What does return@let do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exit the let call and not the function

}
inProgressCount = cursor.getInt(it)
}
}
cursor.close()
return inProgressCount
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ protected Void doInBackground(Void... voids) {
}

AbstractTest[] tests = currentSuite.getTestList(app.getPreferenceManager());
if (tests.length > 0){
if (tests.length > 0) {
publishProgress(START, String.valueOf(suiteIdx));
runTest(currentSuite.getTestList(app.getPreferenceManager()));
}
Expand All @@ -151,6 +151,22 @@ private void runTest(AbstractTest... tests) {
currentTest.run(app, app.getPreferenceManager(),app.getLogger(), app.getGson(), result, i, this);
}
}
/*
* If the result has the more measurements as the number of `nettests`, then the result is done.
* All `nettest` without inputs will produce a singele measurement.
* In this case, the total number of measurements will be equal to the number of `nettests`.
*
* In cases where the `nettest` has inputs, the number of measurements will be equal to or less than the number of inputs.
*
* In the case of `webconnectivity`, the number of measurements will be less than or equal to the number of inputs.
* This is because not all inputs(urls) are run since there is a time limit.
*
* In the case of `stunreachability` and `dnscheck`, the inputs are non-existent, but the total measurement count exceeds the test count.
*/
if (result.countTotalMeasurements() >= tests.length) {
result.is_done = true;
result.save();
}
} catch (Exception e) {
publishProgress(ERR, app.getString(R.string.Modal_Error_CantDownloadURLs));
e.printStackTrace();
Expand Down Expand Up @@ -189,8 +205,8 @@ private void downloadURLs() {
}

List<Url> urls = Lists.transform(
webConnectivity.getUrls(),
url -> new Url(url.getUrl(), url.getCategoryCode(), url.getCountryCode())
webConnectivity.getUrls(),
url -> new Url(url.getUrl(), url.getCategoryCode(), url.getCountryCode())
);
List<String> inputs = Url.saveOrUpdate(urls);

Expand Down
Loading