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

Issue 29411633: Issue 5136 - Add test for AndroidWebRequestResourceWrapper

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks ago by anton
Modified:
7 hours, 38 minutes ago
CC:
René Jeschke
Visibility:
Public.

Description

Issue 5136 - Add test for AndroidWebRequestResourceWrapper

Patch Set 1 #

Total comments: 14

Messages

Total messages: 3
anton
https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-android-tests/build.gradle File libadblockplus-android-tests/build.gradle (right): https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-android-tests/build.gradle#newcode35 libadblockplus-android-tests/build.gradle:35: res.srcDirs = ['res'] now we need some preloaded subscriptions ...
2 weeks ago (2017-04-13 13:46:50 UTC) #1
anton
https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java File libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java (right): https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java#newcode121 libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:121: private void updateSubscriptions() actually it's copy-paste from AdblockEngine but ...
2 weeks ago (2017-04-13 13:51:41 UTC) #2
diegocarloslima
7 hours, 38 minutes ago (2017-04-27 18:57:47 UTC) #3
https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
File
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java
(right):

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:43:
private class TestRequest extends AndroidWebRequest
This inner class can be static

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:61:
private class TestStorage implements AndroidWebRequestResourceWrapper.Storage
This inner class can be static

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:83:
private class TestWrapperListener implements
AndroidWebRequestResourceWrapper.Listener
This inner class can be static

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:121:
private void updateSubscriptions()
On 2017/04/13 13:51:41, anton wrote:
> actually it's copy-paste from AdblockEngine but it would be too heavy to use
> AdblockEngine instead of already existing FilterEngine. I could extract static
> method with filterEngine argument like `updateFilterEngine(FilterEngine
engine)`
> but i think it does not worth it just for testing.

Yeah, since it's just for testing... I think you can keep like this for now

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:185:
public void testIntercepted_Easylist()
Why there's this underscore on the method name? I think you should stick to the
camelCase convention for method declaration

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:192:
public void testIntercepted_AcceptableAds()
Same here, I think you should stick to the camelCase convention for method
declaration

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/AndroidWebRequestResourceWrapperTest.java:199:
public void testIntercepted_OnceOnly()
Same here, I think you should stick to the camelCase convention for method
declaration

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
File
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/UtilsTest.java
(right):

https://codereview.adblockplus.org/29411633/diff/29411634/libadblockplus-andr...
libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/UtilsTest.java:57:
public void testUrlWithoutParams_null()
Same for the methods in this class, I think you should stick to the camelCase
convention for method declaration
Sign in to reply to this message.

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