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

optparse: test and allow adjustment of posixly-correct behavior #1049

Merged
merged 7 commits into from Apr 28, 2017

Conversation

Projects
None yet
4 participants
@grondo
Copy link
Contributor

grondo commented Apr 28, 2017

Initially started working on this because I thought POSIXLY_CORRECT behavior of optparse was broken. In order to test properly, an optparse_reset() function was added to allow a full reset of a parser (mostly useful in testing perhaps), then tests for this new interface as well as posixly-correct parsing behavior were added.

In the end, there wasn't a problem with POSIXLY_CORRECT functionality of optparse. Instead of throwing away all the new testing, I added an ability to disable POSIXLY_CORRECT for GNU getopt in optparse, which could come in handy in the future (?).

Additionally, I had to add a fix for #1048 here in order to get travis to pass in my branch.

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Apr 28, 2017

Hm, some of my commits are in a bit of weird state, github says "grondo committed with grondo", maybe different email addresses used to commit and author?

Let me look into that and clean it up before a merge.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 28, 2017

Coverage Status

Coverage increased (+0.08%) to 78.246% when pulling 922caad on grondo:optparse-posixly_correct into 6ea311e on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Apr 28, 2017

Codecov Report

Merging #1049 into master will increase coverage by <.01%.
The diff coverage is 96.96%.

@@            Coverage Diff             @@
##           master    #1049      +/-   ##
==========================================
+ Coverage   77.91%   77.91%   +<.01%     
==========================================
  Files         148      148              
  Lines       25670    25699      +29     
==========================================
+ Hits        20000    20024      +24     
- Misses       5670     5675       +5
Impacted Files Coverage Δ
src/common/liboptparse/optparse.c 89.1% <96.96%> (+0.75%) ⬆️
src/common/libflux/tagpool.c 95.12% <0%> (-1.22%) ⬇️
src/common/libflux/rpc.c 91.2% <0%> (-0.37%) ⬇️
src/broker/overlay.c 67.62% <0%> (-0.33%) ⬇️
src/broker/module.c 82.15% <0%> (-0.29%) ⬇️
src/common/libflux/handle.c 85.6% <0%> (-0.26%) ⬇️
src/modules/kvs/kvs.c 80.92% <0%> (-0.25%) ⬇️
src/bindings/lua/flux-lua.c 81.53% <0%> (-0.1%) ⬇️
src/common/libflux/message.c 81.08% <0%> (ø) ⬆️
src/common/libflux/dispatch.c 83.51% <0%> (+0.27%) ⬆️
... and 2 more
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Apr 28, 2017

Nice bump in test coverage! Ready to merge?

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Apr 28, 2017

Sorry, just coming around to fixing up those commits with strange authorship

grondo added some commits Apr 28, 2017

optparse: add optparse_reset function
Add a function to reset an optparse object along with all its subcommands.
This function zeros option_index, removes all found options and their
arguments, such that the optparse object appears newly created.
optparse/test: test non-option argument handling
Test for posixly-correct non-option argument handling in optparse.
optparse: explicltly pass posixly_correct
In optparse, instead of prepending optstring with "+" to indicate
we always want "POSIXly correct" behavior from GNU getopt, directly
pass the posixly_correct parameter to getopt_internal_r().
optparse: add OPTPARSE_POSIXLY_CORRECT
Allow default posixly correct behavior of optparse to be disabled with
the new OPTPARSE_POSIXLY_CORRECT item, e.g. to disable posixly correct
behavior:

 optparse_set (p, OPTPARSE_POSIXLY_CORRECT, 0);

(Though make sure POSIXLY_CORRECT environment variable is not set
in environment)
optparse/test: test OPTPARSE_POSIXLY_CORRECT
Ensure disabling posixly-correct behavior permutes argv as
described.
travis-ci: lock pylint at v1.5.3
Unknown issues with more recent versions of pylint, so lock in
known-working version 1.5.3.

Fixes #1048

@grondo grondo force-pushed the grondo:optparse-posixly_correct branch from 922caad to 4a129ce Apr 28, 2017

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Apr 28, 2017

Ok, commits fixed up. Assuming travis passes this is ok to go in.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 28, 2017

Coverage Status

Coverage increased (+0.005%) to 78.17% when pulling 4a129ce on grondo:optparse-posixly_correct into 6ea311e on flux-framework:master.

@garlick garlick merged commit e83fd0e into flux-framework:master Apr 28, 2017

4 checks passed

codecov/patch 96.96% of diff hit (target 77.91%)
Details
codecov/project 77.91% (+<.01%) compared to 6ea311e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 78.17%
Details

@grondo grondo deleted the grondo:optparse-posixly_correct branch Jun 1, 2017

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.