Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1967683002:
Test Physical Web upgrade path (Closed)
DescriptionTest Physical Web upgrade path
We used to use a specific SharedPreference file, now we use the common
one. This change tests the upgrade path.
BUG=597316
upgrade
Committed: https://crrev.com/14e576fc9a0dfcfc7e4b49520476cd4079c0cd3e
Cr-Commit-Position: refs/heads/master@{#393054}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase #Patch Set 3 : Return some code lost in the rebase #Patch Set 4 : Remove dead store to local variable #
Total comments: 1
Messages
Total messages: 27 (11 generated)
cco3@chromium.org changed reviewers: + mattreynolds@chromium.org
lgtm
cco3@chromium.org changed reviewers: + nyquist@chromium.org
https://codereview.chromium.org/1967683002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java (right): https://codereview.chromium.org/1967683002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:307: prefs.edit() could there be any race condition here where since we do this in the background, the action would be done multiple times? I guess it's idempotent anyway, so it doesn't matter.
https://codereview.chromium.org/1967683002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java (right): https://codereview.chromium.org/1967683002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:307: prefs.edit() On 2016/05/11 17:05:53, nyquist wrote: > could there be any race condition here where since we do this in the background, > the action would be done multiple times? I guess it's idempotent anyway, so it > doesn't matter. Right, it wouldn't matter.
lgtm
The CQ bit was checked by cco3@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967683002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967683002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by cco3@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org, mattreynolds@chromium.org Link to the patchset: https://codereview.chromium.org/1967683002/#ps40001 (title: "Return some code lost in the rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967683002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967683002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
The CQ bit was checked by cco3@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org, mattreynolds@chromium.org Link to the patchset: https://codereview.chromium.org/1967683002/#ps60001 (title: "Remove dead store to local variable")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967683002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967683002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Test Physical Web upgrade path We used to use a specific SharedPreference file, now we use the common one. This change tests the upgrade path. BUG=597316 upgrade ========== to ========== Test Physical Web upgrade path We used to use a specific SharedPreference file, now we use the common one. This change tests the upgrade path. BUG=597316 upgrade Committed: https://crrev.com/14e576fc9a0dfcfc7e4b49520476cd4079c0cd3e Cr-Commit-Position: refs/heads/master@{#393054} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/14e576fc9a0dfcfc7e4b49520476cd4079c0cd3e Cr-Commit-Position: refs/heads/master@{#393054}
Message was sent while issue was closed.
jbudorick@chromium.org changed reviewers: + jbudorick@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... File chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java (right): https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:182: assertEquals(0, oldPrefs.getInt(arbitrary_key, 0)); This failed on the L tablet bot with the following: C 682.844s Main [FAIL] org.chromium.chrome.browser.physicalweb.UrlManagerTest#testUpgradeFrom1or2: C 682.844s Main junit.framework.AssertionFailedError: expected:<0> but was:<1> C 682.844s Main at org.chromium.chrome.browser.physicalweb.UrlManagerTest.testUpgradeFrom1or2(UrlManagerTest.java:182) C 682.844s Main at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214) C 682.844s Main at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199) C 682.844s Main at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:129) C 682.844s Main at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191) C 682.844s Main at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176) C 682.844s Main at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555) C 682.844s Main at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853) see https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Te...
Message was sent while issue was closed.
On 2016/05/12 00:03:41, jbudorick wrote: > https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... > File > chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java > (right): > > https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... > chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:182: > assertEquals(0, oldPrefs.getInt(arbitrary_key, 0)); > This failed on the L tablet bot with the following: > > C 682.844s Main [FAIL] > org.chromium.chrome.browser.physicalweb.UrlManagerTest#testUpgradeFrom1or2: > C 682.844s Main junit.framework.AssertionFailedError: expected:<0> but was:<1> > C 682.844s Main at > org.chromium.chrome.browser.physicalweb.UrlManagerTest.testUpgradeFrom1or2(UrlManagerTest.java:182) > C 682.844s Main at > android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214) > C 682.844s Main at > android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199) > C 682.844s Main at > org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:129) > C 682.844s Main at > android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191) > C 682.844s Main at > android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176) > C 682.844s Main at > android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555) > C 682.844s Main at > android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853) > > see > https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Te... Thanks, looking into it.
Message was sent while issue was closed.
On 2016/05/12 00:13:11, cco3 wrote: > On 2016/05/12 00:03:41, jbudorick wrote: > > > https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... > > File > > > chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java > > (right): > > > > > https://codereview.chromium.org/1967683002/diff/60001/chrome/android/javatest... > > > chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:182: > > assertEquals(0, oldPrefs.getInt(arbitrary_key, 0)); > > This failed on the L tablet bot with the following: > > > > C 682.844s Main [FAIL] > > org.chromium.chrome.browser.physicalweb.UrlManagerTest#testUpgradeFrom1or2: > > C 682.844s Main junit.framework.AssertionFailedError: expected:<0> but > was:<1> > > C 682.844s Main at > > > org.chromium.chrome.browser.physicalweb.UrlManagerTest.testUpgradeFrom1or2(UrlManagerTest.java:182) > > C 682.844s Main at > > > android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214) > > C 682.844s Main at > > android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199) > > C 682.844s Main at > > org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:129) > > C 682.844s Main at > > android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191) > > C 682.844s Main at > > android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176) > > C 682.844s Main at > > > android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555) > > C 682.844s Main at > > > android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1853) > > > > see > > > https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Te... > > Thanks, looking into it. New change: https://codereview.chromium.org/1969223002 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
