On 09/02/18 2:47 PM, Dominique Devienne wrote:
On Fri, Feb 9, 2018 at 10:06 AM, Jaikiran Pai <jai.forums2...@gmail.com>
wrote:
I need some inputs on how we should go about this specific change/test?
Should this test continue to expect a exception or is it fine to expect
that target to complete cleanly (without copying anything)?
What's the source of the NPE?
If it's an implementation detail, then it's fine to no longer throw IMHO.
We have FileNameMapper interface which has a:
String[] mapFileName(String sourceFileName);
API. The contract/javadoc of this API says:
* Returns an array containing the target filename(s) for the
* given source file.
*
* <p>if the given rule doesn't apply to the source file,
* implementation must return null....
We have an implementation of this interface, the IdentityMapper, whose
implementation so far, IMO, wasn't following this contract. If it was
passed a null source file name, that implementation would return an
String[] with one element and the contained element in that array would
be null.
Callers of this API are expected to understand that the API itself might
return null, so they used to just check the return value and not its
contents. They would then go and start working on these individual
elements in the array and start running into NPEs in a bunch of places.
The change I made was to the implementation of IdentityMapper, so that
it returns null instead of an array with one null element, when the
input to it is null. That way, the callers' logic of checking the return
value for null, is enough for them to skip such resources.
So to me, this does look like something that we should take care off in
that specific test, so that it no longer expects a build exception to be
thrown.
-Jaikiran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
