commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Manuel Müller <themu...@gmail.com>
Subject [cli] getParsedOptionValue
Date Thu, 07 Apr 2011 20:11:38 GMT
Hello,

yesterday i was doing some work with commons-cli and stumbled across
getParsedOptionValue in the CommandLine class.
Being new to this library i was very confused to always get a null value
instead of some string. The problem was that i didn't set the type in my
Option objects.
I would suggest to set the dafult type to String beause thats exactly what i
was expacting. I would have provided a patch but i was not sure where to
implement it (either as a fallback in CommandLine.getParsedOptionValue or in
the Option constructor).

Another thing i noticed were the Unit tests. In many test cases only
PosixParser gets tested, but this class is marked as depracated and the
suggested replacement, DefaultParser, doesn't get testet.
Seing this i immediately thought of the new Junit 4.x features Theory and
Parametrized Test which i wanted to test some time ago. So i ported the
Tests to Junit 4.x (using Junit 4.8.2) and rewrote some of them using these
features.

...
@RunWith(Parameterized.class)
public class ValueTest {
    private CommandLine _cl = null;
    private CommandLineParser parser = null;
    private Options opts = new Options();

    @Parameters
    public static Collection<CommandLineParser[]> data() {
        CommandLineParser[][] data = new CommandLineParser[][] {
                { new PosixParser() }, { new DefaultParser() } };
        return Arrays.asList(data);
    }

    public ValueTest(CommandLineParser parser) {
        this.parser = parser;
    }
...

...
@RunWith(Theories.class)
public class ApplicationTest  {
    @DataPoint public static CommandLineParser POSIX_PARSER = new
PosixParser();
    @DataPoint public static CommandLineParser DEFAULT_PARSER = new
DefaultParser();


    @org.junit.Test
    @Theory
    public void testLs(CommandLineParser parser) throws Exception {
...

If there is interest in these things i would apply this to all test and
provide a patch for it.
Using Junit 4.x woud also remove the dependency to the junit-addons package
because ArrayAssert.assertEquals could be replaced by assertArrayEquals.


Manuel

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message