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