RFR: 8159523. Fix tests depending on absence of -limitmods in VM arguments.
mandy.chung at oracle.com
Thu Mar 9 01:33:44 UTC 2017
> On Mar 8, 2017, at 5:21 PM, Alexandre (Shura) Iline <alexandre.iline at oracle.com> wrote:
>> Also specifying the target explicitly makes the test clearer what it does.
>> .addExports(“java.base/jdk.internal.reflect”, “ALL-UNNAMED”)
> Makes sense.
> I hope though that you are not against having a constant for “ALL-UNNAMED”, such as Task.ALL_UNNAMED. A lot of people, myself included prefer usage of constants over the copy-pasted value.
I am not against defining a constant variable (I see the benefit of doing that to avoid typo). However, Task is not the right class to define ALL_UNNAMED. Another alternative is to introduce a new Modules class to define those strings which seems overkill. A typo will be caught when the test is run and it isn’t too bad.
>> 136 .shouldContain(Task.OutputKind.STDERR, "IllegalAccessError");
>> 164 .shouldContain(Task.OutputKind.STDOUT, "specified more than once”);
>> It may be useful to add “stdoutShouldContain” and “stderrShouldContain” method, like what ProcessTools provides.
> There will be a further discussion on this - Igor will be summarizing it in a separate e-mail. What he will be suggesting is
> or something similar.
That may be fine.
> Yes for filterStandardOption(…).
> I meant ignoreStandardOption(String, int) as a public API. Some options are supported by JavaOptions, but there could be others which are not. But I agree that we can hide it for now - there may never be a case for it.
Sounds good. Add it when we identify a need for it.
>> Should we remove the use of JDKToolFinder since I assume jdk.testlibrary.task will replace jdk.testlibrary.* helper classes in the future.
> Probably also make it private for now. Or remove - we can re-introduce it later.
I suggest to remove it as a clean start.
More information about the core-libs-dev