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

Testing for -h is throwing strange errors #129

Closed
hrj opened this issue Jan 27, 2017 · 2 comments
Closed

Testing for -h is throwing strange errors #129

hrj opened this issue Jan 27, 2017 · 2 comments

Comments

@hrj
Copy link

hrj commented Jan 27, 2017

Firstly, thanks for this useful library!

We have been using scallop in the abandon project for a long time, and it works fine. But when we tried to add a test case for the "-h" option, we are getting strange errors from scalatest.

Please see this PR.

Any idea what's going on? Is the output stream being closed prematurely?

@Rogach
Copy link
Member

Rogach commented Jan 27, 2017

I traced the error to this line: https://github.com/hrj/abandon/blob/master/base/src/main/scala/co/uproot/abandon/Config.scala#L19

Help and Version are not subclasses of ScallopException, they are subclasses of ScallopResult. So the match statement did not catch them, instead forwarding to super.onError - which prints help and calls sys.exit(0), killing the forked process and closing the streams.

Adding separate catch clause for Help fixed the failing test. Also, if you want to retain default behavior for help and version, you should take a look at original implementation of onError: https://github.com/scallop/scallop/blob/develop/src/main/scala/org.rogach.scallop/ScallopConf.scala#L403

Alternative way will be to forbid sys.exit for the time of this test execution, as we do when testing scallop: https://github.com/scallop/scallop/blob/develop/src/test/scala/CapturingTest.scala#L24

By the way, your colleague is right - only --help is created by default, -h needs to be specified explicitly.

@hrj
Copy link
Author

hrj commented Jan 27, 2017

Thanks a lot for the detailed response. We will fixup the onError implementation in abandon. Closing this issue in the meanwhile.

@hrj hrj closed this as completed Jan 27, 2017
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

No branches or pull requests

2 participants