Skip to content

Conversation

@tleonhardt
Copy link
Member

@tleonhardt tleonhardt commented May 21, 2019

cmd2.Cmd.cmdloop() now returns self.exit_code which should be an integer

Also:

  • Refactored examples to call sys.exit(app.cmdloop()) in their __main__
  • Running transcript tests now sets the exit_code accordingly based on success/failure
  • Updated CHANGELOG
  • Updated README
  • Updated Sphinx docs
  • Added unit test for case when transcript test fails
  • Fixed a bug in how line numbers were calculated for transcript tests
  • Refactored output of transcript test failures so that an unhelpful traceback message is no longer printed

Closes #682
Closes #684

cmd2.Cmd.cmdloop() now returns self.exit_code which should be an integer

Also:
- Refactored examples to call sys.exit(app.cmdloop()) in their __main__
- Running transcript tests now sets the exit_code accordingly based on success/failure
- Updated CHANGELOG
- Updated README
- Updated Sphinx docs
- Added unit test for case when transcript test fails
@tleonhardt tleonhardt added this to the 0.9.13 milestone May 21, 2019
@tleonhardt tleonhardt requested review from kmvanbrunt and kotfu May 21, 2019 03:51
@tleonhardt tleonhardt requested a review from anselor as a code owner May 21, 2019 03:51
@tleonhardt tleonhardt self-assigned this May 21, 2019
@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

Merging #685 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #685      +/-   ##
==========================================
+ Coverage   95.28%   95.29%   +0.01%     
==========================================
  Files          11       11              
  Lines        3181     3191      +10     
==========================================
+ Hits         3031     3041      +10     
  Misses        150      150
Impacted Files Coverage Δ
cmd2/cmd2.py 95.58% <100%> (+0.02%) ⬆️
cmd2/transcript.py 93.69% <100%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fd474f...791b226. Read the comment docs.

@teto
Copy link

teto commented May 21, 2019

is there a way to know if we are in transcript testing mode ?

Should I test this PR ?
Note that I currently override cmdloop like this:

    def cmdloop(self, intro=None):
        """
        overrides baseclass just to be able to catch exceptions
        """
        try:
            super().cmdloop()
        except KeyboardInterrupt as e:
            pass

        # Exception raised by sys.exit(), which is called by argparse
        # we don't want the program to finish just when there is an input error
        except SystemExit as e:
            self.cmdloop()
        except mp.MpTcpException as e:
            print(e)
            self.cmdloop()
        except Exception as e:
            log.critical("Unknown error, aborting...")
            log.critical("%s" % e)
            print("Displaying backtrace:\n")
            traceback.print_exc()

I wonder if it will collide with this PR.
I can change it so it's just for reference on how some projects may use cmd2.

@tleonhardt
Copy link
Member Author

tleonhardt commented May 21, 2019

@teto
If self._transcript_files is not None is True then we are in transcript testing mode.

It would be a good idea for you to test this PR since it will affect your application.

You will probably need to change your override slightly to be something like this:

    def cmdloop(self, intro=None):
        """
        overrides baseclass just to be able to catch exceptions
        """
        sys_exit_code = 0
        try:
            sys_exit_code = super().cmdloop()
        except KeyboardInterrupt as e:
            pass
        # Exception raised by sys.exit(), which is called by argparse
        # we don't want the program to finish just when there is an input error
        except SystemExit as e:
            sys_exit_code = self.cmdloop()
        except mp.MpTcpException as e:
            print(e)
            sys_exit_code = self.cmdloop()
        except Exception as e:
            log.critical("Unknown error, aborting...")
            log.critical("%s" % e)
            print("Displaying backtrace:\n")
            traceback.print_exc()

        return sys_exit_code

And then in your outer __main__ call something like:

sys.exit(app.cmdloop())

Also, I'm curious why you are catching this exception: except SystemExit as e:. That should not be necessary when using any of our argparse-based decorators.

@tleonhardt
Copy link
Member Author

I'm going to go ahead and merge this. But please feel free to provide comments, I'd be happy to revise after the fact.

@tleonhardt tleonhardt merged commit d0add87 into master May 21, 2019
@tleonhardt tleonhardt deleted the exit_code branch May 21, 2019 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor exit_code and exception printing for transcript testing Refactor exit_code implementation

4 participants