Using -testid yields misleading error message

[cacraigucar]

The command:
./create_test --xml-machine yellowstone --xml-compiler intel --xml-category aux_cam -testid aux_cam_try8

gives the error message:
ERROR: Could not fill-out test name, partial string 'aux_cam_try8' had no grid information and you did not provide any

[cacraigucar]

This appears to be an issue with using a single dash. When I use a "-compare cam5_4_80" I get basically
the same message:
ERROR: Could not fill-out test name, partial string 'cam5_4_80' had no grid information and you did not provide any

[cacraigucar]

I tested this command again and still get the still "unhelpful" error message

[jgfouca]

@cacraigucar single-dash long-options (-long) are not POSIX compliant and create_test doesn't support them. Use (-s) for short option and (--long) for long options.

[rljacob]

But shouldn't "-testid" result in a message like "invalid option"? Some hint that they're not using POSIX compliant arguments.

[jgfouca]

This is handled by the python argparse module.

[cacraigucar]

The problem is that we are moving from using single dashes with CESM1 to double dashes in CESM2. That combined with the likelihood of a simple typo, it would be good if an error like this could be trapped (in my cases it was a typo that I didn't realize I'd made). I was requesting a simple error message that doesn't say something about grid information which sends the user in a completely wrong direction trying to solve their problem.

[rljacob]

What is handled by argparse? A good error message? Apparently not.

[jedwards4b]

I have a method to address this but need to think a bit about where to put it - basically it needs to happen in each interface prior to calling parser.parse_args

   for arg in args[1:]:
        if arg.startswith("--"):
            next
        if arg.startswith("-") and len(arg) > 2:
            expect(False, "Arguments should begin with -- or be single character (-s)")

[jedwards4b]

Actually

 for arg in args[1:]:
        if arg.startswith("-") and len(arg) > 2:
            expect(False, "Arguments should begin with -- or be single character (-s)")

is sufficient

[jedwards4b]

How's this?

 ./create_test SMS.f19_g16.X -test-id fred
ERROR: Invalid argument -test-id
  Arguments should begin with -- or be single character (-s)
  Use --help for a complete list of available options

[cacraigucar]

Perfect - just what I was hoping for!

[rljacob]

How about:

 ./create_test SMS.f19_g16.X -test-id fred
ERROR: Invalid argument -test-id
  Multi-character arguments should begin with "--" and single character with "-"
  Use --help for a complete list of available options

[jedwards4b]

I have a branch where I have implemented this change, however there is a problem and I think that we need to discuss whether we want this on before cesm2 - namely there is still code internal to cime as well as in the component models which use the single dash long form and so this change will require changes to all of the component models. I think if we are going to apply this change we need to apply it uniformly and not on an option by option basis. Branch is test_cli on my fork - how shall we proceed?

[mvertens]

If there is code in component models that just use a single dash - I think its too late to try to change this across the entire system before the release. There are already enough issues we need to resolve in the next few weeks before the freeze without trying to make this consistent across the system as well.

…

On Wed, Apr 19, 2017 at 4:45 PM, jedwards4b ***@***.***> wrote: I have a branch where I have implemented this change, however there is a problem and I think that we need to discuss whether we want this on before cesm2 - namely there is still code internal to cime as well as in the component models which use the single dash long form and so this change will require changes to all of the component models. I think if we are going to apply this change we need to apply it uniformly and not on an option by option basis. Branch is test_cli on my fork - how shall we proceed? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxExfwd-vX4jLteE7TdrxfczroECpVks5rxnJygaJpZM4JoASB> .

[jedwards4b]

But I think that Cheryl is right - the current error message is confusing. I would like to continue to track the trunk with this branch and consider merging it as part of our beta07 tag.

[mvertens]

Agreed.

…

On Thu, Apr 20, 2017 at 9:34 AM, jedwards4b ***@***.***> wrote: But I think that Cheryl is right - the current error message is confusing. I would like to continue to track the trunk with this branch and consider merging it as part of our beta07 tag. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxE8uh4y-RgBGrVGerB7GD5Z8BjaY4ks5rx179gaJpZM4JoASB> .

[rljacob]

Also the python create_test, the subject of this issue, has always been strict about single dash I think. So it won't break anything to have a better error message/better enforcement in create_test.

[jgfouca]

@jedwards4b @rljacob @mvertens

I feel strongly that we should not be trying to anticipate every way the user may mess-up their command-line arguments. Most of the time, argparse will provide a decent error. In this case, we got unlucky and the error wasn't very good. What happened is this:

-compare cam5_4_80

... this gets interpreted as

-c  ompare cam5_4_80

so that cam5_4_80 is being interpreted as a test name. One thing we could do is to improve the error when an invalid test name is provided.

Since our other tools support non-compliant POSIX long options, maybe we should just do the same in create_test so we're at least consistent.

[cacraigucar]

In this particular case (moving from using single dash to double dash for this release), I suspect that this will be a mistake that will be made more often. It is hard to retrain fingers (at least I had that problem two days in a row!) When I encountered this error message the first time, I spent a lot of time digging into the config_compset file where my grid is defined, trying to figure out what I did wrong there. It was only when I typed the command from scratch (without the typo) that I was able to figure out what I did wrong.