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

Python tests ported #12

Closed
wants to merge 1 commit into from
Closed

Conversation

VorontsovIE
Copy link

Old test moved to test_docopt_rb.rb (class DocoptTest renamed to DocoptRbTest) -
but they fails due to absence of example.rb.

Three tests ported from python failed:

  1. Set doesn't glue equal Arguments (#test_set)
  2. docopt parses '[--]' into '--' not true as python implementation (#test_allow_double_underscore)
  3. the same is true for '[-]' (#test_allow_single_underscore)

One test (#test_issue34_unicode_strings) not implemented (it's about unicode)

.

Old test moved to test_docopt_rb.rb (class DocoptTest renamed to DocoptRbTest) -
but they fails due to absence of example.rb.

Three tests ported from python failed:
1) Set doesn't glue equal Arguments (#test_set)
2) docopt parses '[--]' into '--' not true as python implementation (#test_allow_double_underscore)
3) the same is true for '[-]' (#test_allow_single_underscore)

One test (#test_issue34_unicode_strings) not implemented (it's about unicode)
@keleshev
Copy link
Member

This looks really good for me. Also shows that language-agnostic tests have worse test-coverage than Python tests—that means I need to port more Python tests to language-agnostic tests. (Also, I recently discovered coverage.py which I will use to ensure 100% test-coverage of Python and language-agnostic tests).

Good job! I'm not Rubyist so it's best that @shabbyrobe and @njoh take a look at this to decide if this can be merged without changes.

@johari
Copy link
Member

johari commented Sep 23, 2012

Yay!! Thank you @prijutme4ty ! : )
It looks great and merging it should be fine..
I'll check if we can use a ruby code coverage tool along with these tests (and language agnostic ones) this weekend.

Also, automating tests for different ruby versions sounds reasonable to me.. maybe on travis-ci.
opinions?

@shabbyrobe
Copy link
Member

This is fantastic! Thank you very much. I have merged into the develop branch: 00a567d

Are these tests failing for you too?

  • test_allow_double_underscore
  • test_allow_single_underscore
  • test_set

Also, what would be the best practice for including that file that is failing in test_docopt_rb.rb? The file in question is actually called examples/example_options.rb.

@shabbyrobe shabbyrobe closed this Sep 23, 2012
@VorontsovIE
Copy link
Author

Thank you for merging!

Not to forget - there were two test:
#test_count_multiple_flags, #test_count_multiple_commands where I found construction:

assert_raise(Docopt::Exit) {
assert_equal docopt('usage: prog [-vv]', '-vvv')
}

I've easily ported this but can't understand why such a strange syntax used.

@shabbyrobe, yes these 3 tests fails for me too. I've described in commit info, reasons of failure.
Thank you for specifing file that should be used. I'll send a pull request to restore testing functionality in a short time.

@keleshev
Copy link
Member

@prijutme4ty

assert_raise(Docopt::Exit) {
  assert_equal docopt('usage: prog [-vv]', '-vvv')
}

why such a strange syntax used

See docopt/docopt#30. Would be interesting to hear—what do you think of this feature?

@VorontsovIE
Copy link
Author

@halst I think this feature is ok, but I can't realize where I can use it :) Not so many applications use smth like -vvv option.
I meant strange syntax of test, not CLI. I've sent a small patch where I've described what do I mean.

@shabbyrobe
Copy link
Member

Actually there are quite a few applications I've found in my collection of saved help messages that make use of the counted short option. @halst, I think you were right all along though: -v3 makes much more sense. Oh well, it's in there now, is there any cost to keeping it in?

@keleshev
Copy link
Member

I think we should definitely keep it, because (as I said earlier) this functionality fills gap in syntax interpretation—how to interpret ... for flags and repeating flags. Unless there is some better interpretation—we should keep this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants