On Tue, 16 Nov 2021 17:43:42 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
> After having looked at this change, I think I might have fallen prey to the
> famous "XY problem". Instead of asking on how to emulate error conditions in
> a black-box fashion first, I asked to provide white-box support for those
> through JavadocTester.
I didn't see the question this way, and I thought the JBS issue was well stated.
Setting aside `JavadocTester` for a moment, there were two ways to solve the
general problem of triggering exceptions to emulate weird conditions.
1. Build back doors into the main system `JavaFileManager` ... i..e.
`JavacFileManager` (note `Javac`), such that the code can be triggered by
backdoor access, either reflective, or breaking abstraction or system
properties.
2. Provide a new file manager to do the job.
I prefer not to do the first ... I prefer not to build those backdoors into the
main file manager that is routinely used everywhere. So that means to do the
second.
Providing a new file manager to do the job means either using
`javax.tools.DocumentationTool` directly, or else updating `JavadocTester`. The
latter seems more desirable because of all the extra code in
`JavadocTester.javadoc(...)` surrounding the actual tool invocation. And it
comes at the cost of tweaking the tool invocation. It would be possible to
have `JavadocTester` invoke `Start` directly without modification, but that
would have required modifying _all_ existing tests, because of the need to use
`com.sun.tools.javac.util.Context`. Stepping away from that means modifying
either `Main` or `Start` to accept a file manager directly, and of the two, it
seemed more future-proof to adapt `Main` into a builder-style API, without
otherwise changing any existing code paths.
The one surprise/disappointment was the "sort-of technical debt" I discovered
in `Start` ... that file-manager options in the "command-line" args were
ignored if a non-standard file manager was used. That seemed like a worthwhile
issue to address here.
In terms of providing a custom file manager, the question is, "which file
objects" and "which methods". I went through a series of iterations before
deciding on using `Predicate<JavaFileObject>` to filter the file objects. For a
while, the API used regular expressions applied to the name of the file object,
but that runs into issues with platform-specific file separators. Yeah, we
could use the URI, but equally, it's nicer to have access to the file object
itself, for filtering directly on properties of the file object, including the
possibility of `.equals()`. As for "which methods", yeah, you suggested the
two methods used to _read_ objects, but what if we want to write tests where
the exceptions are thrown when _writing_ the object.
That all being said, I accept that the API as presented at this point may seem
to be a sledgehammer, and it may be convenient to provide a coat of paint. I
like the general underlying architecture of the solution, but maybe we can come
up with a method on the builder along the following lines:
static StandardJavaFileManager
throwExceptionWhenReading(StandardJavaFileManager fm, JavaFileObject jfo)
We can bike-shed on the name, and maybe there's a 3rd argument
`Function<JavaFileObject, Throwable>` to create the exception to supply the
exception that will be thrown. And, there's clearly a family of possible
overloads, such as replacing the `JavaFileObject` parameter with a
`Predicate<JavaFileObject>` to make it easier to specify the behavior for more
than one file object, if that should be useful, or `Predicate<String>` to
specify a predicate on the file name, or last component of the filename.
To summarize, I don't think the change is as scary as it might appear at first,
although it may be somewhat lacking in ease-of-use for common cases. While the
code of the builder may seem complicated, with its use of reflection and
proxies, the entire class, the file is only 240 lines, including the legal
header block and documentation comments.
> Just to make sure we're on the same page and aren't trying to solve a more
> complex problem than is required. Is there an easy way to make these methods
> throw: javax.tools.JavaFileManager#getFileForInput and
> javax.tools.FileObject#getCharContent?
TL;DR: Yes. But I didn't know about the `getFileForInput` use case. I'll look
to make that possible.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6404
