RFR: 8288783: Error messages are confusing when options conflict in -XX:StartFlightRecording [v3]

Chihiro Ito cito at openjdk.org
Thu Feb 9 03:46:53 UTC 2023


On Sat, 15 Oct 2022 09:03:38 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

>> Chihiro Ito has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   reverting file headers
>
> What worries me a bit is the testing. It is thorough, but perhaps too thorough? 
> 
> It might fail due to other reasons than the added logic, or it may become a maintenance burden. For example, the flush-interval option is planned to be removed. Starting 40 JVMs to verify an improved error message seems excessive. 
> 
> I would put the tests in one file, TestConflictingOptions and use options that are harmless (disk and name), but include "settings" as it should be repeatable, i.e 
> 
>     void testJCmdConflict() {
>       var output= JcmdHelper.jcmd("JFR.start name=hello name=greetings");
>       output.shouldContain("name can only be specified once")
>     }
> 
>     void testStartFlightRecordingConflict() {
>      var output = ProcessTools.executeTestJava("-XX:StartFlightRecording:disk=true,disk=false,name=cat,name=dog");
>      output.shouldContain("disk and name can only be specified once.")
>     }
> 
>     void testSettings() {
>      var output = ProcessTools.executeTestJava("-XX:StartFlightRecording:settings=default,settings=profile");
>      output.shouldNotContain("settings can only be specified once")
>     }
> 
> This should give 100% line coverage and make sure it works from both command line and jcmd.

@egahlin Thanks for the review.

-------------

PR: https://git.openjdk.org/jdk/pull/9302


More information about the hotspot-jfr-dev mailing list