RFR (smallish?) 8061611: Remove deprecated command line flags

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Tue Dec 16 11:04:11 UTC 2014


Looks good!

One question, I'm fine with the change regardless of the answer; Both tests uses 
makeOptionArg() and it is a generic method that could be useful in other tests 
as well. Would it make sense to move it into the testlibrary?
/Jesper


Derek White skrev 15/12/14 23:24:
> FYI -
>
> New webrev shows splitting regression test into simpler alias and deprecation
> tests...
>
> http://cr.openjdk.java.net/~drwhite/8061611/webrev.03/
>
>   - Derek
>
> On 12/15/14 1:04 PM, Derek White wrote:
>> Thanks Bengt - comments below...
>>
>> On 12/15/14 7:51 AM, Bengt Rutisson wrote:
>>> Hi Derek,
>>>
>>> On 2014-12-12 15:35, Derek White wrote:
>>>> This is a request for final re-review.
>>>>
>>>> Updated for comments, and added regression test for arguments. This test is
>>>> a bit overkill for this bug, but it's extensible for other deprecated and
>>>> aliased options.
>>>>
>>>> Re-merged with tree.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~drwhite/8061611/webrev.01/
>>>
>>> The code changes look good to me.
>>>
>>> A few comments about the test.
>>>
>>> I don't think it is worth testing that the removed flags produce error
>>> messages. Starting a VM takes some time and doing it 13 times just to check
>>> that no one by mistake added the flags back seems like a waste of resources
>>> to me. The risk that the flags are suddenly added back is almost zero, right?
>> OK.
>>> I am also not convinced that a general and extensible test is the way to go
>>> here. I think I would prefer a more specific test for the flags that were
>>> deprecated now. For tests I think it is more important that the tests are
>>> easily readable and understandable than that code duplication is avoided.
>>> Thus, I strongly prefer small but specific tests that clearly communicates
>>> what went wrong when they fail and are easy to understand how they failed.
>>> So, in this case maybe we should even have two tests?
>>> TestDeprecatedMarkStackFlags and TestDeprecatedConcGCThreadsFlags.
>>
>> OK. I'm trying to calibrate how much testing is appropriate and how it should
>> be organized. In fact, the other organizing principle was to be more "process
>> oriented" - one (or more) regression tests per bug fixed. And regression tests
>> would be essentially immutable - never expanded to test new cases. Is that
>> about right?
>>
>> So to make the tests simpler, but bug specific, how about I break them up for
>> one test to test aliases, and one test to test deprecation warnings?
>>> Instead of parsing the PrintFlagsFinal output you could use
>>> ManagementFactoryHelper.getDiagnosticMXBean().getVMOption() to get the option
>>> values and check if they are set correctly. I think that will reduce the
>>> number of lines in the test further.
>>
>> PrintFlagsFinal has the added benefit of noting that a flag was explicitly set
>> vs. has a default value. (":=" vs "="). I first used bizzare option values to
>> try to test that, but ":=" is definitive.
>>
>> BTW, a motivation for the alias tests was thinking ahead to the redo of
>> "alias" options that I'm planning. I'll want to test that the new alias
>> handling code actually works for the remaining aliased options. But that can
>> get it's own test file.
>>
>> Thanks again!
>>
>>  - Derek
>



More information about the hotspot-gc-dev mailing list