Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(554)

Issue 29819555: Fix libadblockplus-android tests

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by René Jeschke
Modified:
2 days, 11 hours ago
Reviewers:
anton
CC:
jens, diegocarloslima
Visibility:
Public.

Description

Fix libadblockplus-android tests

Patch Set 1 #

Total comments: 6

Patch Set 2 : Added missing files, removed a whitespace #

Total comments: 21

Patch Set 3 : Fixed indentation issues. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+638 lines, -524 lines) Patch
A adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java View 1 2 1 chunk +95 lines, -0 lines 1 comment Download
A + adblock-android-tests/src/org/adblockplus/libadblockplus/BaseJsEngineTest.java View 1 1 chunk +2 lines, -6 lines 0 comments Download
A + adblock-android-tests/src/org/adblockplus/libadblockplus/BasePlatformTest.java View 1 1 chunk +4 lines, -2 lines 0 comments Download
A adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java View 1 2 1 chunk +178 lines, -0 lines 0 comments Download
D adblock-android-tests/src/org/adblockplus/libadblockplus/LazyWebRequest.java View 1 chunk +0 lines, -32 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/MockFilterChangeCallback.java View 2 chunks +18 lines, -3 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/NoOpUpdaterCallback.java View 1 chunk +8 lines, -0 lines 0 comments Download
A + adblock-android-tests/src/org/adblockplus/libadblockplus/UpdaterTest.java View 1 1 chunk +11 lines, -8 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java View 8 chunks +23 lines, -48 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestTest.java View 6 chunks +38 lines, -23 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/AppInfoJsObjectTest.java View 4 chunks +7 lines, -9 lines 0 comments Download
D adblock-android-tests/src/org/adblockplus/libadblockplus/tests/BaseFilterEngineTest.java View 1 chunk +0 lines, -56 lines 0 comments Download
D adblock-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java View 1 chunk +0 lines, -32 lines 0 comments Download
D adblock-android-tests/src/org/adblockplus/libadblockplus/tests/BasePlatformTest.java View 1 chunk +0 lines, -69 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/ConsoleJsObjectTest.java View 2 chunks +5 lines, -2 lines 0 comments Download
A adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineFirstRunTest.java View 1 1 chunk +70 lines, -0 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java View 1 2 5 chunks +53 lines, -20 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineUpdaterTest.java View 2 chunks +17 lines, -5 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/GlobalJsObjectTest.java View 1 chunk +1 line, -0 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/IsAllowedConnectionCallbackTest.java View 7 chunks +30 lines, -42 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/JsEngineTest.java View 1 chunk +1 line, -0 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/JsTest.java View 1 chunk +1 line, -0 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/MockWebRequestTest.java View 1 2 4 chunks +31 lines, -74 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/NotificationTest.java View 6 chunks +10 lines, -13 lines 0 comments Download
M adblock-android-tests/src/org/adblockplus/libadblockplus/tests/UpdateCheckTest.java View 11 chunks +35 lines, -38 lines 0 comments Download
D adblock-android-tests/src/org/adblockplus/libadblockplus/tests/UpdaterTest.java View 1 chunk +0 lines, -42 lines 0 comments Download

Messages

Total messages: 18
René Jeschke
2 months, 1 week ago (2018-07-11 13:37:04 UTC) #1
anton
On 2018/07/11 13:37:04, René Jeschke wrote: What do you think about using mockito or powermock?
2 months, 1 week ago (2018-07-11 13:57:31 UTC) #2
anton
https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java (right): https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java#newcode56 adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java:56: int prev = filterEngine.getListedFilters().size(); extra not required space
2 months, 1 week ago (2018-07-11 13:58:31 UTC) #3
anton
On 2018/07/11 13:58:31, anton wrote: > https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > File > adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > (right): > > ...
2 months, 1 week ago (2018-07-11 14:02:44 UTC) #4
anton
https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java (right): https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java#newcode32 adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java:32: public class FilterEngineTest extends BaseFilterEngineTest i was unable to ...
2 months ago (2018-07-16 06:58:53 UTC) #5
anton
On 2018/07/16 06:58:53, anton wrote: > https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > File > adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > (right): > > ...
1 month, 4 weeks ago (2018-07-23 07:13:46 UTC) #6
René Jeschke
On 2018/07/23 07:13:46, anton wrote: > On 2018/07/16 06:58:53, anton wrote: > > > https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java ...
1 month ago (2018-08-16 11:47:06 UTC) #7
René Jeschke
https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java (right): https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java#newcode32 adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java:32: public class FilterEngineTest extends BaseFilterEngineTest On 2018/07/16 06:58:52, anton ...
1 month ago (2018-08-16 11:47:15 UTC) #8
René Jeschke
On 2018/08/16 11:47:15, René Jeschke wrote: > https://codereview.adblockplus.org/29819555/diff/29819556/adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > File > adblock-android-tests/src/org/adblockplus/libadblockplus/tests/FilterEngineTest.java > (right): > ...
1 month ago (2018-08-16 12:03:06 UTC) #9
René Jeschke
On 2018/08/16 12:03:06, René Jeschke wrote: > On 2018/08/16 11:47:15, René Jeschke wrote: > > ...
1 month ago (2018-08-16 12:13:56 UTC) #10
René Jeschke
Complete patch set is up.
1 month ago (2018-08-16 12:28:24 UTC) #11
anton
rene, could you please add to "CC" diego and jens too (i have no edit ...
3 weeks, 5 days ago (2018-08-24 12:25:04 UTC) #12
René Jeschke
https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java (right): https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java#newcode18 adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java:18: package org.adblockplus.libadblockplus; On 2018/08/24 12:25:03, anton wrote: > Why ...
3 weeks, 1 day ago (2018-08-27 20:02:16 UTC) #13
René Jeschke
PS to the 'patterns.ini' issue. https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java (right): https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java#newcode147 adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java:147: File patterns = new ...
3 weeks, 1 day ago (2018-08-27 20:16:37 UTC) #14
anton
On 2018/08/27 20:02:16, René Jeschke wrote: > https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java > File > adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java > (right): > ...
3 weeks, 1 day ago (2018-08-28 07:03:55 UTC) #15
anton
On 2018/08/27 20:16:37, René Jeschke wrote: > PS to the 'patterns.ini' issue. > > https://codereview.adblockplus.org/29819555/diff/29857583/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseTest.java ...
3 weeks, 1 day ago (2018-08-28 07:08:19 UTC) #16
anton
https://codereview.adblockplus.org/29819555/diff/29866603/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java File adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java (right): https://codereview.adblockplus.org/29819555/diff/29866603/adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java#newcode86 adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java:86: filterEngine.getListedSubscriptions().get(0).removeFromList(); isn't it better to return `boolean` for `removeFromList` ...
2 days, 11 hours ago (2018-09-17 06:55:14 UTC) #17
anton
2 days, 11 hours ago (2018-09-17 06:56:10 UTC) #18
On 2018/09/17 06:55:14, anton wrote:
>
https://codereview.adblockplus.org/29819555/diff/29866603/adblock-android-tes...
> File
>
adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java
> (right):
> 
>
https://codereview.adblockplus.org/29819555/diff/29866603/adblock-android-tes...
>
adblock-android-tests/src/org/adblockplus/libadblockplus/BaseFilterEngineTest.java:86:
> filterEngine.getListedSubscriptions().get(0).removeFromList();
> isn't it better to return `boolean` for `removeFromList` call (actually
removed
> or nor)? Probably it requires changes in `libadblockplus` module  though and
can
> be done in separate task..

in general LGTM now (minor note about checking if subscription was actually
removed).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5