Niedzielski has uploaded a new change for review.
https://gerrit.wikimedia.org/r/259923
Change subject: Hygiene: fix SyntaxHighlighter/Test
......................................................................
Hygiene: fix SyntaxHighlighter/Test
• Fix always false instanceof test assertions:
✗ assertThat(result.getClass(), instanceOf(Expected.class))
✓ assertThat(result, instanceOf(Expected.class));
• Move callback invocation to main thread. In general, UI callbacks are
expected to be invoked on the main thread.
• Move test's anonymous inline callback to a static nested class that
just exposes a latch interface. Inline creation blows the method
count and increases the indentation level which decreases readability
and often discourage DRYness.
• Use test's new latched callback to just wait until the AsyncTask is
done and then perform assertions on the data in the test thread.
Assertion failures in AsyncTasks seem to actually kill the runner
instead of just failing a single test. I'm not 100% sure on this.
• Add @VisibleForTesting to OnSyntaxHighlightListener so the reader
knows this is test-only code. Bubble the interface to the top of the
class so it's better delineated from the data.
• Replace Activity parameter with ContextThemeWrapper. CTW is needed to
get themed attributes but Activities are heavy to test.
• Initialize handler member prior to setting a listener that could
invoke it.
• Drop onCatch logging and isCancelled check. These are already done in
SaneAsyncTask.
Change-Id: I8ba3c2d014e6a1afa021d17fb40d2c9ad8775d2a
---
M
app/src/androidTest/java/org/wikipedia/editing/richtext/SyntaxHighlighterTest.java
M app/src/main/java/org/wikipedia/editing/richtext/SyntaxHighlighter.java
2 files changed, 60 insertions(+), 54 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/23/259923/1
diff --git
a/app/src/androidTest/java/org/wikipedia/editing/richtext/SyntaxHighlighterTest.java
b/app/src/androidTest/java/org/wikipedia/editing/richtext/SyntaxHighlighterTest.java
index 839570f..4628311 100644
---
a/app/src/androidTest/java/org/wikipedia/editing/richtext/SyntaxHighlighterTest.java
+++
b/app/src/androidTest/java/org/wikipedia/editing/richtext/SyntaxHighlighterTest.java
@@ -1,53 +1,62 @@
package org.wikipedia.editing.richtext;
-import android.app.Activity;
import android.support.annotation.NonNull;
-import android.support.test.rule.ActivityTestRule;
+import android.view.ContextThemeWrapper;
import android.widget.EditText;
-import org.junit.Rule;
import org.junit.Test;
-import org.wikipedia.page.PageActivity;
+import org.wikipedia.R;
+import org.wikipedia.testlib.TestLatch;
import java.util.List;
+import static android.support.test.InstrumentationRegistry.getTargetContext;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
public class SyntaxHighlighterTest {
- @Rule
- @NonNull
- public final ActivityTestRule<PageActivity> activityRule = new
ActivityTestRule<>(PageActivity.class);
-
@Test
- public void testSyntaxHighlight() throws Exception {
-
+ public void testSyntaxHighlight() throws Throwable {
final String testStr = "foo {{template1}} bar {{template2}} baz";
final int span1Start = 4;
final int span1End = 17;
final int span2Start = 22;
final int span2End = 35;
- EditText editText = new EditText(getActivity());
- new SyntaxHighlighter(getActivity(), editText,
- new SyntaxHighlighter.OnSyntaxHighlightListener() {
- @Override
- public void syntaxHighlightResults(List<SpanExtents> spanExtents) {
- assertThat(spanExtents.size(), is(2));
- assertThat(spanExtents.get(0).getClass(),
instanceOf(ColorSpanEx.class));
- assertThat(spanExtents.get(0).getStart(), is(span1Start));
- assertThat(spanExtents.get(0).getEnd(), is(span1End));
- assertThat(spanExtents.get(1).getClass(),
instanceOf(ColorSpanEx.class));
- assertThat(spanExtents.get(1).getStart(), is(span2Start));
- assertThat(spanExtents.get(1).getEnd(), is(span2End));
- }
- });
-
+ Callback callback = new Callback();
+ EditText editText = new EditText(getContext());
+ new SyntaxHighlighter(getContext(), editText, callback);
editText.setText(testStr);
+
+ List<SpanExtents> result = callback.await();
+ assertThat(result.size(), is(2));
+ assertThat(result.get(0), instanceOf(ColorSpanEx.class));
+ assertThat(result.get(0).getStart(), is(span1Start));
+ assertThat(result.get(0).getEnd(), is(span1End));
+ assertThat(result.get(1), instanceOf(ColorSpanEx.class));
+ assertThat(result.get(1).getStart(), is(span2Start));
+ assertThat(result.get(1).getEnd(), is(span2End));
}
- private Activity getActivity() {
- return activityRule.getActivity();
+ @NonNull
+ private ContextThemeWrapper getContext() {
+ return new ContextThemeWrapper(getTargetContext(),
R.style.Theme_Light);
+ }
+
+ private static class Callback implements
SyntaxHighlighter.OnSyntaxHighlightListener {
+ private final TestLatch latch = new TestLatch();
+ private List<SpanExtents> result;
+
+ @Override
+ public void syntaxHighlightResults(List<SpanExtents> spanExtents) {
+ result = spanExtents;
+ latch.countDown();
+ }
+
+ public List<SpanExtents> await() throws Throwable {
+ latch.await();
+ return result;
+ }
}
}
diff --git
a/app/src/main/java/org/wikipedia/editing/richtext/SyntaxHighlighter.java
b/app/src/main/java/org/wikipedia/editing/richtext/SyntaxHighlighter.java
index 7d67d25..3abadda 100644
--- a/app/src/main/java/org/wikipedia/editing/richtext/SyntaxHighlighter.java
+++ b/app/src/main/java/org/wikipedia/editing/richtext/SyntaxHighlighter.java
@@ -1,18 +1,20 @@
package org.wikipedia.editing.richtext;
-import android.app.Activity;
import android.graphics.Color;
import android.graphics.Typeface;
import android.os.Handler;
import android.os.Looper;
+import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
import android.text.Editable;
import android.text.Spanned;
import android.text.TextWatcher;
import android.text.format.DateUtils;
-import android.util.Log;
+import android.view.ContextThemeWrapper;
import android.widget.EditText;
import org.wikipedia.R;
import org.wikipedia.concurrency.SaneAsyncTask;
+import org.wikipedia.util.log.L;
import java.util.ArrayList;
import java.util.List;
@@ -21,7 +23,10 @@
import static org.wikipedia.util.ResourceUtil.getThemedAttributeId;
public class SyntaxHighlighter {
- private static String TAG = "SyntaxHighlighter";
+ @VisibleForTesting
+ interface OnSyntaxHighlightListener {
+ void syntaxHighlightResults(List<SpanExtents> spanExtents);
+ }
private EditText textBox;
private List<SyntaxRule> syntaxRules;
@@ -30,15 +35,14 @@
private Handler handler;
private OnSyntaxHighlightListener syntaxHighlightListener;
- public interface OnSyntaxHighlightListener {
- void syntaxHighlightResults(List<SpanExtents> spanExtents);
+
+ public SyntaxHighlighter(ContextThemeWrapper context, EditText textBox) {
+ this(context, textBox, null);
}
- public SyntaxHighlighter(final Activity parentActivity, final EditText
textBox) {
- this(parentActivity, textBox, null);
- }
-
- public SyntaxHighlighter(final Activity parentActivity, final EditText
textBox, OnSyntaxHighlightListener listener) {
+ public SyntaxHighlighter(final ContextThemeWrapper context,
+ final EditText textBox,
+ @Nullable OnSyntaxHighlightListener listener) {
this.textBox = textBox;
this.syntaxHighlightListener = listener;
syntaxRules = new ArrayList<>();
@@ -51,7 +55,7 @@
syntaxRules.add(new SyntaxRule("{{", "}}", new
SyntaxRule.SyntaxRuleStyle() {
@Override
public SpanExtents createSpan(int spanStart, SyntaxRule
syntaxItem) {
- return new
ColorSpanEx(parentActivity.getResources().getColor(getThemedAttributeId(parentActivity,
R.attr.syntax_highlight_template_color)),
+ return new
ColorSpanEx(context.getResources().getColor(getThemedAttributeId(context,
R.attr.syntax_highlight_template_color)),
Color.TRANSPARENT, spanStart,
syntaxItem);
}
}));
@@ -60,7 +64,7 @@
syntaxRules.add(new SyntaxRule("[[", "]]", new
SyntaxRule.SyntaxRuleStyle() {
@Override
public SpanExtents createSpan(int spanStart, SyntaxRule
syntaxItem) {
- return new
ColorSpanEx(parentActivity.getResources().getColor(getThemedAttributeId(parentActivity,
R.attr.link_color)),
+ return new
ColorSpanEx(context.getResources().getColor(getThemedAttributeId(context,
R.attr.link_color)),
Color.TRANSPARENT, spanStart,
syntaxItem);
}
}));
@@ -69,7 +73,7 @@
syntaxRules.add(new SyntaxRule("[", "]", new
SyntaxRule.SyntaxRuleStyle() {
@Override
public SpanExtents createSpan(int spanStart, SyntaxRule
syntaxItem) {
- return new
ColorSpanEx(parentActivity.getResources().getColor(getThemedAttributeId(parentActivity,
R.attr.link_color)),
+ return new
ColorSpanEx(context.getResources().getColor(getThemedAttributeId(context,
R.attr.link_color)),
Color.TRANSPARENT, spanStart,
syntaxItem);
}
}));
@@ -78,7 +82,7 @@
syntaxRules.add(new SyntaxRule("<", ">", new
SyntaxRule.SyntaxRuleStyle() {
@Override
public SpanExtents createSpan(int spanStart, SyntaxRule
syntaxItem) {
- return new
ColorSpanEx(parentActivity.getResources().getColor(R.color.syntax_highlight_htmltag),
+ return new
ColorSpanEx(context.getResources().getColor(R.color.syntax_highlight_htmltag),
Color.TRANSPARENT, spanStart,
syntaxItem);
}
}));
@@ -139,6 +143,8 @@
}));
*/
+ handler = new Handler(Looper.getMainLooper());
+
// add a text-change listener that will trigger syntax highlighting
// whenever text is modified.
textBox.addTextChangedListener(new TextWatcher() {
@@ -157,8 +163,6 @@
handler.postDelayed(syntaxHighlightCallback,
DateUtils.SECOND_IN_MILLIS / 2);
}
});
-
- handler = new Handler(Looper.getMainLooper());
}
private Runnable syntaxHighlightCallback = new Runnable() {
@@ -261,17 +265,15 @@
}
}
- if (syntaxHighlightListener != null) {
- syntaxHighlightListener.syntaxHighlightResults(spansToSet);
- }
return spansToSet;
}
@Override
public void onFinish(List<SpanExtents> result) {
- if (isCancelled()) {
- return;
+ if (syntaxHighlightListener != null) {
+ syntaxHighlightListener.syntaxHighlightResults(result);
}
+
// TODO: probably possible to make this more efficient...
// Right now, on longer articles, this is quite heavy on the UI
thread.
// remove any of our custom spans from the previous cycle...
@@ -285,12 +287,7 @@
textBox.getText().setSpan(spanEx, spanEx.getStart(),
spanEx.getEnd(), Spanned.SPAN_INCLUSIVE_INCLUSIVE);
}
time = System.currentTimeMillis() - time;
- Log.d(TAG, "That took " + time + "ms");
- }
-
- @Override
- public void onCatch(Throwable caught) {
- Log.d(TAG, caught.getMessage());
+ L.v("That took " + time + "ms");
}
}
--
To view, visit https://gerrit.wikimedia.org/r/259923
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8ba3c2d014e6a1afa021d17fb40d2c9ad8775d2a
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
