Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2878523002:
SimpleCache: Add a test that covers recovery from missing index. (Closed)
|
Created:
3 years, 7 months ago by Maks Orlovich Modified:
3 years, 7 months ago Reviewers:
jkarlin CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org, net-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionSimpleCache: Add a test that covers recovery from missing index.
(As I can't seem to find a preexisting one)
BUG=
Review-Url: https://codereview.chromium.org/2878523002
Cr-Commit-Position: refs/heads/master@{#470947}
Committed: https://chromium.googlesource.com/chromium/src/+/31a646c7268cb0957b681ede8332da9bdc513bd2
Patch Set 1 #Patch Set 2 : Fix race, so it actually exercises the restore path consistently. #
Dependent Patchsets: Messages
Total messages: 19 (10 generated)
The CQ bit was checked by morlovich@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by morlovich@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
morlovich@chromium.org changed reviewers: + jkarlin@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm!
The CQ bit was checked by morlovich@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494512612052980,
"parent_rev": "b99daebfd25fdf8346fcef1110afd26bd971308a", "commit_rev":
"31a646c7268cb0957b681ede8332da9bdc513bd2"}
Message was sent while issue was closed.
Description was changed from ========== SimpleCache: Add a test that covers recovery from missing index. (As I can't seem to find a preexisting one) BUG= ========== to ========== SimpleCache: Add a test that covers recovery from missing index. (As I can't seem to find a preexisting one) BUG= Review-Url: https://codereview.chromium.org/2878523002 Cr-Commit-Position: refs/heads/master@{#470947} Committed: https://chromium.googlesource.com/chromium/src/+/31a646c7268cb0957b681ede8332... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/31a646c7268cb0957b681ede8332...
Message was sent while issue was closed.
Just got e-mail that this flaked on "on Android Cronet KitKat Builder", with: C 335.081s Main [ RUN ] DiskCacheBackendTest.SimpleCacheIndexRecovery C 335.081s Main ../../net/disk_cache/backend_unittest.cc:3912: Failure C 335.081s Main Value of: simple_cache_impl_->index()->init_method() C 335.081s Main Actual: 1 C 335.081s Main Expected: disk_cache::SimpleIndex::INITIALIZE_METHOD_NEWCACHE C 335.081s Main Which is: 2 I think I'll practice my reverting, for now.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2879613002/ by morlovich@chromium.org. The reason for reverting is: Seem to break (or maybe just flake?) on "Android Cronet KitKat Builder". Need to figure out how to test it myself like that, I guess. .
Message was sent while issue was closed.
On 2017/05/11 at 15:29:02, morlovich wrote: > A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2879613002/ by morlovich@chromium.org. > > The reason for reverting is: Seem to break (or maybe just flake?) on "Android Cronet KitKat Builder". > > Need to figure out how to test it myself like that, I guess. > > . There're instructions for Android build here: https://chromium.googlesource.com/chromium/src/+/lkcr/docs/android_build_inst... Note the "Convert existing Linux checkout" if that's what you're using. I'd think you can escape with running the unittests on an emulator vs using an actual Android device.
Message was sent while issue was closed.
On 2017/05/11 16:02:56, whywhat wrote: > On 2017/05/11 at 15:29:02, morlovich wrote: > > A revert of this CL (patchset #2 id:20001) has been created in > https://codereview.chromium.org/2879613002/ by mailto:morlovich@chromium.org. > > > > The reason for reverting is: Seem to break (or maybe just flake?) on "Android > Cronet KitKat Builder". > > > > Need to figure out how to test it myself like that, I guess. > > > > . > > There're instructions for Android build here: > https://chromium.googlesource.com/chromium/src/+/lkcr/docs/android_build_inst... > Note the "Convert existing Linux checkout" if that's what you're using. > I'd think you can escape with running the unittests on an emulator vs using an > actual Android device. Oh, yeah, I've done that before, but had trouble with the test framework trying to talk to the JVM to figure out paths and stuff.
Message was sent while issue was closed.
On 2017/05/11 at 16:13:33, morlovich wrote: > On 2017/05/11 16:02:56, whywhat wrote: > > On 2017/05/11 at 15:29:02, morlovich wrote: > > > A revert of this CL (patchset #2 id:20001) has been created in > > https://codereview.chromium.org/2879613002/ by mailto:morlovich@chromium.org. > > > > > > The reason for reverting is: Seem to break (or maybe just flake?) on "Android > > Cronet KitKat Builder". > > > > > > Need to figure out how to test it myself like that, I guess. > > > > > > . > > > > There're instructions for Android build here: > > https://chromium.googlesource.com/chromium/src/+/lkcr/docs/android_build_inst... > > Note the "Convert existing Linux checkout" if that's what you're using. > > I'd think you can escape with running the unittests on an emulator vs using an > > actual Android device. > > Oh, yeah, I've done that before, but had trouble with the test framework trying to talk to the JVM > to figure out paths and stuff. You might need to run build/install-build-deps-android.sh or something? Also sourcing build/android/envsetup.sh sets up path for adb and so on. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
