Move UrlManagerTest to junit

Move UrlManagerTest to junit

UrlManagerTest does not need to be an instrumentation test, so this
change moves it from javatests/ to junit/.

BUG=696804
Review-Url: https://codereview.chromium.org/2723783003
Cr-Commit-Position: refs/heads/master@{#456804}
Committed: https://chromium.googlesource.com/chromium/src/+/cbb4ab906568a80aa3e7a314e6570cc3b8900777

[cco3, 2017-02-28 21:29:09]

cco3@chromium.org changed reviewers:
+ mattreynolds@chromium.org

[cco3, 2017-02-28 21:29:09]

Hi jbudorick@, Could you take a look at UrlManagerTest.testUpgradeFromNone?

My condition code doesn't work.  Is there a standard way of waiting on writes to
SharedPreferences?

[jbudorick, 2017-03-02 16:53:05]

On 2017/02/28 21:29:09, cco3 wrote:
> Hi jbudorick@, Could you take a look at UrlManagerTest.testUpgradeFromNone?
> 
> My condition code doesn't work.  Is there a standard way of waiting on writes
to
> SharedPreferences?

Sorry about the delay here! I've been really busy over the last few days but
will try to get to this today.

[jbudorick, 2017-03-08 04:55:40]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2017-03-08 04:55:40]

https://codereview.chromium.org/2723783003/diff/1/chrome/android/junit/src/or...
File
chrome/android/junit/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java
(right):

https://codereview.chromium.org/2723783003/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:350:
condition.signal();
Apologies again about the delay. I applied your patch locally & gave this a try.
It looks like the issue is that you aren't acquiring the lock before attempting
to signal or wait on the associated condition, resulting in a bunch of
IllegalMonitorStateExceptions.

I was able to get your test to pass by adding locking around both operations,
i.e.

  lock.lock();
  try {
    condition.signal()
  } finally {
    lock.unlock();
  }

(with a similar addition around the assertTrue(condition.await(...)) call below)

[cco3, 2017-03-09 19:11:59]

On 2017/03/08 04:55:40, jbudorick wrote:
>
https://codereview.chromium.org/2723783003/diff/1/chrome/android/junit/src/or...
> File
>
chrome/android/junit/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java
> (right):
> 
>
https://codereview.chromium.org/2723783003/diff/1/chrome/android/junit/src/or...
>
chrome/android/junit/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:350:
> condition.signal();
> Apologies again about the delay. I applied your patch locally & gave this a
try.
> It looks like the issue is that you aren't acquiring the lock before
attempting
> to signal or wait on the associated condition, resulting in a bunch of
> IllegalMonitorStateExceptions.
> 
> I was able to get your test to pass by adding locking around both operations,
> i.e.
> 
>   lock.lock();
>   try {
>     condition.signal()
>   } finally {
>     lock.unlock();
>   }
> 
> (with a similar addition around the assertTrue(condition.await(...)) call
below)

OK, if this is the right approach, I'll take it.  I was hoping there would be
something analogous to CriteriaHelper.pollInstrumentationThread.

[mattreynolds, 2017-03-09 19:52:06]

lgtm

[cco3, 2017-03-09 20:08:44]

cco3@chromium.org changed reviewers:
+ dtrainor@chromium.org

[cco3, 2017-03-09 20:08:44]

Hi David, would you be able to review this change?

[David Trainor- moved to gerrit, 2017-03-14 15:02:47]

lgtm

[cco3, 2017-03-14 17:42:11]

The CQ bit was checked by cco3@chromium.org

[cco3, 2017-03-14 17:42:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, mattreynolds@chromium.org
Link to the patchset: https://codereview.chromium.org/2723783003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-14 17:42:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cco3, 2017-03-14 18:37:05]

The CQ bit was checked by cco3@chromium.org

[cco3, 2017-03-14 18:37:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, mattreynolds@chromium.org
Link to the patchset: https://codereview.chromium.org/2723783003/#ps60001
(title: "Add @Test to disabled test")

[commit-bot: I haz the power, 2017-03-14 18:37:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-14 20:10:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489516625178690,
"parent_rev": "1a9bed2a008dc2a98fa5f3af900a6bad79e5947f", "commit_rev":
"cbb4ab906568a80aa3e7a314e6570cc3b8900777"}

[commit-bot: I haz the power, 2017-03-14 20:11:04]

Description was changed from

==========
Move UrlManagerTest to junit

UrlManagerTest does not need to be an instrumentation test, so this
change moves it from javatests/ to junit/.

BUG=696804
==========

to

==========
Move UrlManagerTest to junit

UrlManagerTest does not need to be an instrumentation test, so this
change moves it from javatests/ to junit/.

BUG=696804

Review-Url: https://codereview.chromium.org/2723783003
Cr-Commit-Position: refs/heads/master@{#456804}
Committed:
https://chromium.googlesource.com/chromium/src/+/cbb4ab906568a80aa3e7a314e657...
==========

[commit-bot: I haz the power, 2017-03-14 20:11:05]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/cbb4ab906568a80aa3e7a314e657...

[tzik, 2017-03-15 07:52:01]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2755643002/ by tzik@chromium.org.

The reason for reverting is: The tests in this CL got flaky after the move:
https://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/3...
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FAndroid...

failures:
org.chromium.chrome.browser.physicalweb.UrlManagerTest#testUpgradeFromNone
org.chromium.chrome.browser.physicalweb.UrlManagerTest#testSerializationWorksWithPoorlySerializedResult

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertTrue(Assert.java:52)
	at
org.chromium.chrome.browser.physicalweb.UrlManagerTest.testSerializationWorksWithPoorlySerializedResult(UrlManagerTest.java:291)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertTrue(Assert.java:52)
	at
org.chromium.chrome.browser.physicalweb.UrlManagerTest.testUpgradeFromNone(UrlManagerTest.java:361)
.