Hi Babak, Thanks for the review, I just updated the code with some suggestion of you. For the SSLContextParametersTest it is caused by the different JDKs handles the SSL related setting differently, I just updated the code to avoid the null collection returned.
-- Willem Jiang Red Hat, Inc. Web: http://www.redhat.com Blog: http://willemjiang.blogspot.com (http://willemjiang.blogspot.com/) (English) http://jnn.iteye.com (http://jnn.javaeye.com/) (Chinese) Twitter: willemjiang Weibo: 姜宁willem On Tuesday, November 19, 2013 at 3:59 AM, Babak Vahdat wrote: > Hi > > Just spotted couple of things on the commit mailing list of today: > > http://git-wip-us.apache.org/repos/asf/camel/diff/000581e6 > > This commit now makes the following asserts passing: > > assertStartsWith((String[]) null, "foo"); > assertStartsWith(new String[] {}, "foo"); > assertStartsWith((Collection<String>) null, "foo"); > assertStartsWith(new ArrayList<String>(), "foo"); > > Which is actually wrong as that would be wrong to claim that e.g. the > String "foo" is an element of a given empty collection. Other than that > this change does not actually fix the failing test as the problem is > something else, namely there're elements inside the given array/collection > which do NOT start with the given prefix. So maybe a better approach would > be to change the asserts from: > > assertTrue(value.startsWith(prefix)); > > to: > > assertTrue("'" + value + "' does not start with the prefix: '" + prefix > + "'!", value.startsWith(prefix)); > > So we get an idea about what actually is going wrong on ubuntu. See also > the note inside the checkProtocols() method of this test class. > > http://git-wip-us.apache.org/repos/asf/camel/diff/8a080c97 > > - The following getters are missing: getProvider() / getSchemaLocation() / > getSchemaLocations() for the properties being newly introduced. > - Instead of: > > @SuppressWarnings("rawtypes") > JSONProvider jsonProvider = new JSONProvider(); > > We could better do: > > JSONProvider<Object> jsonProvider = new JSONProvider<Object>(); > > - There's no benefit of the following generic List type newly introduced > by the API: > > public void setProviders(List<? extends Object> providers)... > > As <?> is as good as <? extends Object> so maybe better do: > > public void setProviders(List<?> providers)... > > See http://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.4 > > ...Every type variable declared as a type parameter has a bound. If no > bound is declared for a type variable, Object is assumed. > > Babak
