RFR: JDK-8199352: The Jib artifact resolver in test lib needs to print better error messages (original) (raw)

Igor Ignatyev igor.ignatyev at oracle.com
Fri Mar 9 00:12:08 UTC 2018

Looks good to me.

-- Igor

On Mar 8, 2018, at 4:06 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:

On 2018-03-08 15:24, Igor Ignatyev wrote: Hi Erik, Thanks for looking at this! to avoid incompatibility, you could have just made ArtifactResolverException a subclass of java.io.FileNotFoundException. This is correct, but I don't think the exception should be of type FileNotFoundException. IMO, this is something different and trying to re-purpose an existing exception type is rarely a good idea. it seems you forgot to add ArtifactResolverException.java file to the repo. Doh! Added. a minor nit: in JibArtifactManager::newInstance, you pass "Could not resolve " + JIBSERVICEFACTORY to ClassNotFoundException constructor. by the convention, the message in CNFE is the classname. Right, good point, fixed. New Webrev: http://cr.openjdk.java.net/~erikj/8199352/webrev.02/ /Erik -- Igor

On Mar 8, 2018, at 2:08 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:

The Jib artifact resolver is not very good at telling us why things go wrong. The reason is that it swallows exceptions. This patch changes the API from throwing a FileNotFoundException, which I don't really think fits correctly in all cases, to a new API specific exception. I have greped for all uses of this API in the tests and changed the exception type caught at the caller location. I verified that I didn't break anything by compiling all the affected test classes and by running some of them for a bit. With these changes it should be easier to diagnose problems with resolving artifacts in the future. Bug: https://bugs.openjdk.java.net/browse/JDK-8199352 Webrev: http://cr.openjdk.java.net/~erikj/8199352/webrev.01/index.html /Erik

More information about the build-dev mailing list