Refactor exit_code and exception printing for transcript testing

[teto]

I am trying to clean up my program and fixing my tests.
Even when a test fails, the program returns 0. I wonder if it's my fault or cmd2's.
I got rid of my do_quit/cmdloop overrides but even so it fails.

also cmd2 displays a stacktrace wwhile running the tests.
The addition of slashes in the test output is also annoying, can't we get rid of them somehow ?

I run my test from this branch https://github.com/teto/mptcpanalyzer/tree/next like this:

set -e
mptcpanalyzer --test tests/trans_tcp.txt

and the output

(ins)[nix-shell:~/mptcpanalyzer]$ mptcpanalyzer --test tests/trans_tcp.txt
Setting log level to ERROR
Run `checkhealth` in case of issues
>>> examples/server_2_filtered.pcapng
ran cmd ['tshark', '-E', 'header=y', '-r', '/home/teto/mptcpanalyzer/examples/server_2_filtered.pcapng', '-E', 'separator=|', '-o', 'gui.column.format:"Time","%At","ipsrc","%s","ipdst","%d"', '-o', 'tcp.analyze_sequence_numbers:True', '-o', 'mptcp.analyze_mappings:True', '-o', 'mptcp.relative_sequence_numbers:True', '-o', 'mptcp.intersubflows_retransmission:True', '-o', 'mptcp.analyze_mptcp:True', '-2', '-R', 'mptcp or tcp and not icmp', '-T', 'fields', '-e', 'frame.number', '-e', 'frame.time_relative', '-e', 'frame.time_epoch', '-e', '_ws.col.ipsrc', '-e', '_ws.col.ipdst', '-e', 'ip.src_host', '-e', 'ip.dst_host', '-e', 'tcp.stream', '-e', 'tcp.srcport', '-e', 'tcp.dstport', '-e', 'tcp.window_size', '-e', 'tcp.flags', '-e', 'tcp.option_kind', '-e', 'tcp.seq', '-e', 'tcp.len', '-e', 'tcp.ack', '-e', 'tcp.options.timestamp.tsval', '-e', 'tcp.options.timestamp.tsecr', '-e', 'mptcp.expected_token', '-e', 'mptcp.stream', '-e', 'tcp.options.mptcp.sendkey', '-e', 'tcp.options.mptcp.recvkey', '-e', 'tcp.options.mptcp.recvtok', '-e', 'tcp.options.mptcp.datafin.flag', '-e', 'tcp.options.mptcp.subtype', '-e', 'tcp.options.mptcp.rawdataseqno', '-e', 'tcp.options.mptcp.rawdataack', '-e', 'tcp.options.mptcp.subflowseqno', '-e', 'tcp.options.mptcp.datalvllen', '-e', 'tcp.options.mptcp.addrid', '-e', 'mptcp.rawdsn64', '-e', 'mptcp.ack', '-e', 'mptcp.dsn', '-e', 'mptcp.related_mapping', '-e', 'mptcp.reinjection_of', '-e', 'mptcp.reinjected_in']
stderr= 
>>> 0 Server
          packetid  reltime           abstime     ipsrc     ipdst  ... reinjection_of reinjected_in  mptcpdest  tcpdest                 hash
packetid                                                           ...                                                                      
1                1 5.147771 1529916720.856525  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN      NaN  7549266080176862213
2                2 5.166072 1529916720.874826  10.0.0.2  10.0.0.1  ...            NaN           NaN        NaN      NaN  7592325442603492667
3                3 5.347371 1529916721.056124  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN      NaN  7327281565805257113
4                4 5.360296 1529916721.069050  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN      NaN  5422233975304775303
5                5 5.381406 1529916721.090160  10.0.0.2  10.0.0.1  ...            NaN           NaN        NaN      NaN -7604440424514793773

[5 rows x 39 columns]
>>> 0 Client
          packetid  reltime           abstime     ipsrc     ipdst  ... reinjection_of reinjected_in  mptcpdest  tcpdest                 hash
packetid                                                           ...                                                                      
1                1 5.147771 1529916720.856525  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN        2  7549266080176862213
2                2 5.166072 1529916720.874826  10.0.0.2  10.0.0.1  ...            NaN           NaN        NaN        1  7592325442603492667
3                3 5.347371 1529916721.056124  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN        2  7327281565805257113
4                4 5.360296 1529916721.069050  10.0.0.1  10.0.0.2  ...            NaN           NaN        NaN        2  5422233975304775303
5                5 5.381406 1529916721.090160  10.0.0.2  10.0.0.1  ...            NaN           NaN        NaN        1 -7604440424514793773

[5 rows x 39 columns]
>>> 
F
======================================================================
FAIL: runTest (cmd2.cmd2.Cmd.run_transcript_tests.<locals>.TestMyAppCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/nix/store/qfr22rbvklflgysqic5qsl09rnrs85jl-python3.7-cmd2-0.9.12/lib/python3.7/site-packages/cmd2/transcript.py", line 53, in runTest
    self._test_transcript(fname, transcript)
  File "/nix/store/qfr22rbvklflgysqic5qsl09rnrs85jl-python3.7-cmd2-0.9.12/lib/python3.7/site-packages/cmd2/transcript.py", line 110, in _test_transcript
    self.assertTrue(re.match(expected, result, re.MULTILINE | re.DOTALL), message)
AssertionError: None is not true : 
File tests/trans_tcp.txt, line 10
Command was:
list_tcp_connections

Expected:
1\ tcp\ connection\(s\)\
tcp\.stream\ 0:\ 10\.0\.0\.1:33782\ \ \->\ 10\.0\.0\.2:05201\ \

Got:
8 tcp connection(s)
10.0.0.1:33782 -> 10.0.0.2:05201
10.0.0.1:33784 -> 10.0.0.2:05201
10.0.0.1:54595 -> 11.0.0.2:05201
10.0.0.1:57491 -> 11.0.0.2:05201
11.0.0.2:59555 -> 11.0.0.1:05201
11.0.0.1:50077 -> 11.0.0.2:05201
11.0.0.1:35589 -> 10.0.0.2:05201
11.0.0.1:50007 -> 10.0.0.2:05201



----------------------------------------------------------------------
Ran 1 test in 1.076s

FAILED (failures=1)

(ins)[nix-shell:~/mptcpanalyzer]$ echo $?
0

[teto]

forgot to say it's with 0.9.12, I haven't tested master

[tleonhardt]

@teto
Running a cmd2 app in the transcript-based testing mode is not the same as a normally running a cmd2 app. I don't believe we ever attempted to do anything to set the exit code when running transcript tests, so it should always be 0 even when tests fail. Maybe we should change that?

We always print a traceback on failure, but that traceback isn't particularly useful for the user, so perhaps we should remove it and replace it with a more generic "Failure" message (in red).

In regards to slashes in the output, they are used to delimit regular expressions embedded within the transcript and at least in some cases are absolutely essential.

[teto]

Running a cmd2 app in the transcript-based testing mode is not the same as a normally running a cmd2 app. I don't believe we ever attempted to do anything to set the exit code when running transcript tests, so it should always be 0 even when tests fail. Maybe we should change that?

In interactive mode, i understand not wanting to exit but I would vote for changing the exit code in transcript-based testing mode otherwise I have no way to automate testing ? My intent is to use it on the CI so I need to be able to detect failure.

We always print a traceback on failure, but that traceback isn't particularly useful for the user, so perhaps we should remove it and replace it with a more generic "Failure" message (in red).

I don't have much experience with the transcript based system but throwing exceptions at library users does not seem like the correct choice. Maybe hide it behind a flag (even the exisiting debug one ?)

In regards to slashes in the output, they are used to delimit regular expressions embedded within the transcript and at least in some cases are absolutely essential.

I can live with them

[teto]

tested and it seems to work fine. There seems to be color around the failure (which I didn't remember before ?) that I kinda like.
Just curious any ETA for the next release ?

[tleonhardt]

Yes, failure messages now print in red and no longer include the useless trace back.

The next release will probably be within the next two weeks - so soon. I definitely want the renaming of load to run_script in before we release. The history changes would be nice, but those can wait until the release after.