Provide more context information when unable to read input during instrumentation

[Godin]

In #400 we updated Analyzer, but not Instrumenter.

So that for example for the same broken.zip Analyzer.analyzeAll produces:

java.io.IOException: Error while analyzing /tmp/jacoco/in/broken.zip@brokenentry.txt.

while Instrumenter.instrumentAll:

java.util.zip.ZipException: invalid distance too far back

This difference is also visible between Ant tasks report and instrument when they used for archives that contain corrupted nested archives - in a stack trace in case of report name of a nested corrupted archive is shown, while in case of instrument not.

[marchof]

@Godin Why do we need "class" prefix here? Typically the name will be the file name (Example.class) anyways. In case of classes nested in archives this will lead to locations like "archive.jar@class Example.zip". I would prefer not adding a prefix like in Analyzer.

[Godin]

@marchof Here is a story: at first I also wanted to remove word "class", then realized that Instrumenter is used by CoverageTransformer and CoverageTransformerTest expects word "class", where BTW name is not a name of file but a name of class and so without extension .class. After that instead of removal I pulled this word up from instrumentError into existing places of usage instrumentError, preserving existing error messages. And then added new usages of instrumentError without word "class" into places where name does not refer to a class.

Also "archive.jar@class something" is impossible - word "class" can't appear in the middle, because its addition happens at the very end when name is already fully constructed, and its addition happens only in places where we know that name refers to a class.

And so alternatively there are two other options - move addition of prefix "class" into CoverageTransformer or completely remove it. WDYT?

[marchof]

@Godin Let's keep things simple and remove "class" also in the test.

[Godin]

@marchof just to clarify: so accepting that agent will generate message without word "class" ?

[marchof]

@Godin Absolutely! In the context of the agent and the class name it should be obvious that classes get instrumented.

[Godin]

@marchof done. Note that I also did other changes to cover remaining missing cases.

[mathjeff]

Hey, thanks for this

[marchof]

@Godin I propose a minor change to remove duplicated code.

[marchof]

I think we can delegate to the overloaded method here:

public void instrument(final InputStream input, final OutputStream output,
		final String name) throws IOException {
	output.write(instrument(input, name));
}

[Godin]

@marchof indeed... and done

[Godin]

@marchof BTW while working on this I noticed that Javadocs states

IOException - if reading data from the stream fails or the class can't be instrumented

but what about case when write fails? Also was wondering how consumers such as Ant Task behave in such case...

[marchof]

@Godin I was thinking the same. But there will be a specific IOException naming the file which cannot be written. Generating the class data itself is already protected within the instrument() method.