RFR: JDK-8220639 Need a way to augment JTREG_LAUNCHER_OPTIONS from command-line

[David Holmes]

On 16/06/2020 10:35 pm, Magnus Ihse Bursie wrote:
> On 2020-06-16 04:58, David Holmes wrote:
>> Hi Magnus,
>>
>> Great to see this getting fixed! Thanks!
>>
>> On 16/06/2020 1:56 am, Magnus Ihse Bursie wrote:
>>> It turned out that it was not possible to send arguments directly to 
>>> the java that launches jtreg, something that was needed, especially 
>>> to change the output limit using Djavatest.maxOutputSize.
>>>
>>> This patch adds JTREG=LAUNCHER_OPTIONS=xxx to pass generic options to 
>>> the launcher.
>>
>> Is there some special prefix handling going on here? The makefile 
>> lists JTREG_LAUNCHER_OPTIONS as the variable. Without any other 
>> information that is what I would set on the command-line.
> Yes, there is. That's what the ParseKeywordVariable macro does. You 
> *can* set JTREG_LAUNCHER_OPTIONS directly, and it will work as you would 
> assume. But make will produce a warning that it cannot verify that it is 
> correct. If you use the JTREG=LAUNCHER_OPTIONS=xxx syntax, make will 
> verify that it is a valid control variable. So JTREG=LAUNCHER_OPTS=xxx 
> would result in an error, while JTREG_LAUNCHER_OPTS  would just be 
> silently ignored.
> 
> I think this is pretty well explained in doc/testing.md.

Yes - mea culpa - I had not read that.

Thanks,
David

>>
>>> Also, for the specific case of changing the max output size, it adds 
>>> JTREG=MAX_OUTPUT=nnn.
>>
>> Is it worthwhile special-casing for max output? How would someone even 
>> think to ask if such a special case exists? Typical scenario is that 
>> someone sees the output overflow in a jtr file and the message to 
>> increase -Djavatest.maxOutputSize, so they will then ask "how do I 
>> pass a flag to jtreg?" and hopefully easily discover LAUNCHER_OPTIONS 
>> and use it.
> Ok, maybe it isn't. I got the impression from the bug report that this 
> was the main cause for wanting to pass options to the jtreg launcher, 
> and something that a developer "often" do. If it does not make sense to 
> have it, it's better that I remove it, than push a feature that no-one 
> is going to use.
>>
>>> All these new options made me realize there's no way apart from 
>>> reading the code to know what options are available, so I added a 
>>> help system, and documented this in "make help". Now you can get:
>>>
>>> $ make test JTREG=help
>>> Building target 'test' in configuration 'linux-x64'
>>> Valid keywords for JTREG:
>>> JOBS TIMEOUT_FACTOR FAILURE_HANDLER_TIMEOUT TEST_MODE ASSERT VERBOSE 
>>> RETAIN MAX_MEM RUN_PROBLEM_LISTS RETRY_COUNT MAX_OUTPUT OPTIONS 
>>> JAVA_OPTIONS VM_OPTIONS KEYWORDS EXTRA_PROBLEM_LISTS AOT_MODULES 
>>> LAUNCHER_OPTIONS.
>>> RunTests.gmk:292: *** Re-run without 'help' to continue. Stop.
>>
>> That's goods to see but it is unclear what form the values for these 
>> keywords should take, or how to specify multiple keywords. 
> It's all documented in doc/building.[md|html].
> 
>> How do we map these keywords back to jtreg flags?
> Not all map directly to jtreg flags; they all pertain to how RunTest 
> runs the jtreg tests. Some map more or less straight to jtreg options, 
> but far from all.
> 
> Most of these are documented in doc/building.md. They really should all 
> be. I'll go another round on this webrev and make sure the documentation 
> is up to date.
> 
>> I see Jon's separate thread on documenting flags, so is the answer 
>> that jtreg docs will describe all this?
> There's a difference between what jtreg documentation describes (jtreg 
> "bare bones") and what the build documentation describes (how to run 
> OpenJDK testing, both with jtreg and other test suites).
> 
> /Magnus
>>
>> Thanks,
>> David
>> -----
>>
>>> Finally, I fixed some broken comments regarding AOT that I noticed 
>>> while fixing this.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8220639
>>> WebRev: 
>>> http://cr.openjdk.java.net/~ihse/JDK-8220639-add-jtreg-launcher-options/webrev.01 
>>>
>>>
>>> /Magnus
>