Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(214)

Issue 1649543002: Add Unit Tests to SnackbarCollection (Closed)

Created:
4 years, 11 months ago by Ian Wen
Modified:
4 years, 10 months ago
Reviewers:
newt (away)
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@snackbar_notification
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add Unit Tests to SnackbarCollection In this CL, SnackbarCollection is pulled out from SnackbarManager and is tested by JUnit test. All public methods in SnackbarCollection are covered in the unit tests. This CL also fixes a bug in removeMatchingSnackbars() that was found while writing the tests. Hooray! BUG=579347 Committed: https://crrev.com/276c80ef8a8b060e60400f85c6b411bd3e257b79 Cr-Commit-Position: refs/heads/master@{#372277}

Patch Set 1 #

Total comments: 15

Patch Set 2 : Use Mockito #

Total comments: 2

Patch Set 3 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+296 lines, -99 lines) Patch
M chrome/android/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java View 1 1 chunk +130 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java View 1 3 chunks +1 line, -99 lines 0 comments Download
A chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java View 1 2 1 chunk +164 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
Ian Wen
PTAL.
4 years, 11 months ago (2016-01-27 22:24:15 UTC) #2
newt (away)
Thanks for writing unit tests! A few comments inline. https://codereview.chromium.org/1649543002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java File chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java (right): https://codereview.chromium.org/1649543002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java#newcode15 chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java:15: ...
4 years, 10 months ago (2016-01-28 17:32:00 UTC) #3
Ian Wen
All addressed. Mockito is indeed very handy to use. :P https://codereview.chromium.org/1649543002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java File chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java (right): https://codereview.chromium.org/1649543002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarCollection.java#newcode15 ...
4 years, 10 months ago (2016-01-29 02:38:43 UTC) #4
newt (away)
lgtm after comment Looks good! https://codereview.chromium.org/1649543002/diff/20001/chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java File chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java (right): https://codereview.chromium.org/1649543002/diff/20001/chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java#newcode77 chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java:77: assertEquals("Action snackbar should ", ...
4 years, 10 months ago (2016-01-29 02:49:01 UTC) #5
Ian Wen
https://codereview.chromium.org/1649543002/diff/20001/chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java File chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java (right): https://codereview.chromium.org/1649543002/diff/20001/chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java#newcode77 chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java:77: assertEquals("Action snackbar should ", actionBar, collection.getCurrent()); On 2016/01/29 02:49:01, ...
4 years, 10 months ago (2016-01-29 02:53:32 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1649543002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1649543002/40001
4 years, 10 months ago (2016-01-29 02:53:55 UTC) #9
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 10 months ago (2016-01-29 03:34:59 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2016-01-29 03:36:04 UTC) #12
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/276c80ef8a8b060e60400f85c6b411bd3e257b79
Cr-Commit-Position: refs/heads/master@{#372277}

Powered by Google App Engine
This is Rietveld 408576698