Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect default value #1733

Closed
Pochatkin opened this issue Jul 7, 2022 · 11 comments
Closed

Incorrect default value #1733

Pochatkin opened this issue Jul 7, 2022 · 11 comments

Comments

@Pochatkin
Copy link

The test provided shows that, under certain conditions, incorrect clearing of command option values occurs.
test2 fails because after test1 execution Picocli start providing --option option as default value for next executions.

@MicronautTest
class PicocliBugTest {
    @Inject
    private ApplicationContext context;

    private CommandLine cmd;

    private StringWriter sout;

    private StringWriter serr;

    private int exitCode = Integer.MIN_VALUE;

    @BeforeEach
    public void setUp() {
        cmd = new CommandLine(Command.class, new MicronautFactory(context));
        sout = new StringWriter();
        serr = new StringWriter();
        cmd.setOut(new PrintWriter(sout));
        cmd.setErr(new PrintWriter(serr));
    }

    @Test
    public void test1() {
        cmd.execute("--option", "option");
        Assertions.assertEquals("option", sout.toString());
    }

    @Test
    public void test2() {
        cmd.execute();
        Assertions.assertEquals("", sout.toString());
    }

    @CommandLine.Command(name = "command")
    @Singleton
    static class Command implements Runnable {
        @Option(names = "--option")
        private String option;

        @Spec
        CommandSpec spec;


        @Override
        public void run() {
            spec.commandLine().getOut().print(option);
            spec.commandLine().getOut().flush();
        }
    }
}
@remkop
Copy link
Owner

remkop commented Jul 7, 2022

I believe the problem is in the test.
Yes, the test fails, but not because default values are not being reset.
When I run the above test, I get this result:

org.junit.ComparisonFailure: 
Expected :      <-- the expectation of empty string is incorrect, the default is null
Actual   :null  <-- the String value "null" (new StringWriter().print(null).toString() gives the String "null")

I believe the test below is clearer.
Can you run this test below and show the result?
(I commented out the Micronaut stuff and I am using JUnit 4 in the test below, feel free to change to your preferences.)

The below test passes for me, clearing of command option values seems to work fine...

import org.junit.Before;
import org.junit.Test;
import picocli.CommandLine;
import picocli.CommandLine.Option;

import static org.junit.Assert.*;

//@MicronautTest
public class PicocliBugTest {
//    @Inject
//    private ApplicationContext context;

    private CommandLine cmd;

    PicocliBugTest.Command userObject;

    @Before
    public void setUp() {
        cmd = new CommandLine(PicocliBugTest.Command.class/*, new MicronautFactory(context)*/);
        userObject = cmd.getCommand();
    }

    @Test
    public void test1() {
        cmd.execute("--option", "option");
        assertEquals("option", userObject.option);
    }

    @Test
    public void test2() {
        cmd.execute();
        assertEquals("Expected option to be reset to null", null, userObject.option);
        assertNull("Expected option to be reset to null", userObject.option);
    }

    @CommandLine.Command(name = "command")
    //@Singleton
    static class Command implements Runnable {
        @Option(names = "--option")
        private String option;

        @Override
        public void run() {
            System.out.printf("Option='%s'%n", option);
        }
    }
}

@Pochatkin
Copy link
Author

Hello. Thank you for your answer!
Yeah, your test example is completely passed, but this is not the same as I posted.
So, in my case I have Micronaut infrostucrute and I can't just remove it. So, my test is just simple sample of problem which I faced and my question is why when I used MicronautFactory as CommandLine.IFactory implementation I have a problem with value cleaning (Please just uncomment commented lines in your example).
This is bug or just side effect of Micronaut staff which should be interpret as feature? Mby I just smth missed, could you please clarify this moment.

@remkop
Copy link
Owner

remkop commented Jul 11, 2022

Okay that is interesting! That does sound like it may be a Micronaut issue, but I will try to find time to take a look.

Can you do me a favor and post the output of running my test (after re-enabling the Micronaut logic) with system property -Dpicocli.trace=DEBUG please?

@Pochatkin
Copy link
Author

2022-07-11 14:32:36:631 +0300 [INFO][main][DefaultEnvironment] Established active environments: [test]
[picocli DEBUG] Creating CommandSpec for class com.test.PicocliBugTest$Command with factory io.micronaut.configuration.picocli.MicronautFactory
[picocli DEBUG] Getting a com.test.PicocliBugTest$Command instance from factory io.micronaut.configuration.picocli.MicronautFactory@5eeedb60
[picocli DEBUG] Factory returned a com.test.PicocliBugTest$Command instance (4a1c0752)
[picocli INFO] Picocli version: 4.6.2, JVM: 11.0.14.1 (Amazon.com Inc. OpenJDK 64-Bit Server VM 11.0.14.1+10-LTS), OS: Linux 5.11.0-46-generic amd64
[picocli INFO] Parsing 2 command line args [--option, option]
[picocli DEBUG] Parser configuration: optionsCaseInsensitive=false, subcommandsCaseInsensitive=false, abbreviatedOptionsAllowed=false, abbreviatedSubcommandsAllowed=false, aritySatisfiedByAttachedOptionParam=false, atFileCommentChar=#, caseInsensitiveEnumValuesAllowed=false, collectErrors=false, endOfOptionsDelimiter=--, expandAtFiles=true, limitSplit=false, overwrittenOptionsAllowed=false, posixClusteredShortOptionsAllowed=true, separator=null, splitQuotedStrings=false, stopAtPositional=false, stopAtUnmatched=false, toggleBooleanFlags=false, trimQuotes=false, unmatchedArgumentsAllowed=false, unmatchedOptionsAllowedAsOptionParameters=true, unmatchedOptionsArePositionalParams=false, useSimplifiedAtFiles=false
[picocli DEBUG] (ANSI is disabled by default: systemproperty[picocli.ansi]=null, isatty=false, TERM=null, OSTYPE=null, isWindows=false, JansiConsoleInstalled=false, ANSICON=null, ConEmuANSI=null, NO_COLOR=null, CLICOLOR=null, CLICOLOR_FORCE=null)
[picocli DEBUG] Initializing command 'command' (user object: com.test.PicocliBugTest$Command@4a1c0752): 1 options, 0 positional parameters, 0 required, 0 groups, 0 subcommands.
[picocli DEBUG] Set initial value for field String com.test.PicocliBugTest$Command.option of type class java.lang.String to null.
[picocli DEBUG] [0] Processing argument '--option'. Remainder=[option]
[picocli DEBUG] '--option' cannot be separated into <option>=<option-parameter>
[picocli DEBUG] Found option named '--option': field String com.test.PicocliBugTest$Command.option, arity=1
[picocli DEBUG] 'option' doesn't resemble an option: 0 matching prefix chars out of 1 option names
[picocli INFO] Setting field String com.test.PicocliBugTest$Command.option to 'option' (was 'null') for option --option on Command@4a1c0752
[picocli DEBUG] Applying default values for command 'command'
Option='option'
[picocli DEBUG] Creating CommandSpec for class com.test.PicocliBugTest$Command with factory io.micronaut.configuration.picocli.MicronautFactory
[picocli DEBUG] Getting a com.test.PicocliBugTest$Command instance from factory io.micronaut.configuration.picocli.MicronautFactory@4a29f290
[picocli DEBUG] Factory returned a com.test.PicocliBugTest$Command instance (4a1c0752)
[picocli INFO] Picocli version: 4.6.2, JVM: 11.0.14.1 (Amazon.com Inc. OpenJDK 64-Bit Server VM 11.0.14.1+10-LTS), OS: Linux 5.11.0-46-generic amd64
[picocli INFO] Parsing 0 command line args []
[picocli DEBUG] Parser configuration: optionsCaseInsensitive=false, subcommandsCaseInsensitive=false, abbreviatedOptionsAllowed=false, abbreviatedSubcommandsAllowed=false, aritySatisfiedByAttachedOptionParam=false, atFileCommentChar=#, caseInsensitiveEnumValuesAllowed=false, collectErrors=false, endOfOptionsDelimiter=--, expandAtFiles=true, limitSplit=false, overwrittenOptionsAllowed=false, posixClusteredShortOptionsAllowed=true, separator=null, splitQuotedStrings=false, stopAtPositional=false, stopAtUnmatched=false, toggleBooleanFlags=false, trimQuotes=false, unmatchedArgumentsAllowed=false, unmatchedOptionsAllowedAsOptionParameters=true, unmatchedOptionsArePositionalParams=false, useSimplifiedAtFiles=false
[picocli DEBUG] (ANSI is disabled by default: systemproperty[picocli.ansi]=null, isatty=false, TERM=null, OSTYPE=null, isWindows=false, JansiConsoleInstalled=false, ANSICON=null, ConEmuANSI=null, NO_COLOR=null, CLICOLOR=null, CLICOLOR_FORCE=null)
[picocli DEBUG] Initializing command 'command' (user object: com.test.PicocliBugTest$Command@4a1c0752): 1 options, 0 positional parameters, 0 required, 0 groups, 0 subcommands.
[picocli DEBUG] Set initial value for field String com.test.PicocliBugTest$Command.option of type class java.lang.String to option.
[picocli DEBUG] Applying default values for command 'command'
[picocli DEBUG] defaultValue not defined for field String com.test.PicocliBugTest$Command.option
Option='option'

org.opentest4j.AssertionFailedError: Expected option to be reset to null ==> 
Expected :null
Actual   :option

@remkop
Copy link
Owner

remkop commented Jul 11, 2022

Great, thank you!
It looks like picocli thinks the initial value is "option" after the first test. This may be a picocli issue after all...
I will look into if further.

Meanwhile, as a workaround, can you try defining this option with defaultValue = Option.NULL_VALUE?

@Pochatkin
Copy link
Author

Nope, this is doesn't help.

org.opentest4j.AssertionFailedError: Expected option to be reset to null ==> 
Expected :null
Actual   :option

@remkop
Copy link
Owner

remkop commented Jul 12, 2022

The issue is related to the @Singleton annotation on the PicocliBugTest.Command class.
This makes the Micronaut factory cache and reuse that instance for all tests.

However, for each test, a different CommandLine instance is created!
So, the second CommandLine instance is created and initialized with the result of the first test, which explains why the initial value of the option is the string "option".

This test demonstrates without using Micronaut:

// This test passes and prints:
// Option='option'
// Option='null'
@Test
public void testReuseBothCommandLineAndUserObject() {
    Command myUserObject = new Command();
    CommandLine cmdLine = new CommandLine(myUserObject);
    cmdLine.execute("--option", "option");
    cmdLine.execute();
    assertEquals("Expected option to be reset to null", null, myUserObject.option);
    assertNull("Expected option to be reset to null", myUserObject.option);
}

// This test fails and prints:
// Option='option'
// Option='option'
@Test
public void testReuseUserObjectWithDifferentCommandLine() {
    Command myUserObject = new Command();
    CommandLine cmdLine = new CommandLine(myUserObject);
    cmdLine.execute("--option", "option");  // after this, myUserObject.option has value "option"
    CommandLine cmdLine2 = new CommandLine(myUserObject); // current values of user object become the default value
    cmdLine2.execute();
    assertEquals("Expected option to be reset to null", null, myUserObject.option); // <-- this expectation fails
    assertNull("Expected option to be reset to null", myUserObject.option);
}

So, I believe this issue is an artifact of the test, not a real issue.

Unless your application actually creates new CommandLine instances but keeps the same user object instance?
(Because that will not work correctly...)

@remkop
Copy link
Owner

remkop commented Jul 15, 2022

@Pochatkin did this resolve the issue?

@valepakh
Copy link

valepakh commented Aug 5, 2022

@remkop There's another issue which is similar to this one but manifests itself in the picocli-shell-jline3 based interactive shell and can be seen in the picocli.shell.jline3.example.Example.java

prompt> cmd -v -d 1 -u SECONDS
Hi there. You asked for 1 SECONDS.
prompt> cmd -v
Hi there. You asked for 1 SECONDS.

As I undestand, the CommandLine instance created in the main loop caches user objects.

@remkop
Copy link
Owner

remkop commented Aug 5, 2022

@valepakh oh interesting... It looks like that is a separate issue, that has to do with default values in argument groups. (I don't think it has to do with JLine, any app that reuses the CommandLine instance may encounter this.)

I thought this was covered in the documentation, but you found a case that is not covered. Can you raise a separate ticket for this?

@valepakh
Copy link

valepakh commented Aug 5, 2022

Done #1768

@remkop remkop added the status: feedback-provided 💬 Feedback has been provided label Aug 18, 2022
@remkop remkop closed this as completed Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants