Backslashes get stripped from regexes used in transcript testing

[tleonhardt]

With the way parsing in Cmd2TestCase is implemented, it means that any transcript with regular expressions containing backslashes will not behave as expected. Backslashes in regexes are very common. It would be much better if a much wider variety of regexes worked properly.

[kotfu]

I have some background with regexes, so I'll take a crack at this one. I've never used pyparsing before, so we'll see how I do. I'll holler if I need some help or guidance.

[kotfu]

The current code (and documentation) is very clear that the transcript testing ignores whitespace in the comparison. The documentation states you can use regular expressions, but says 'they should not include any whitespace expressions.'

The code does (almost) exactly what the documentation says, but there are many non-obvious implications that even a user with lots of experience with regular expressions may not grasp.

The code uses MULTILINE regular expression matching, which means the special character '$' matches 'the end of the string or just before the newline at the end of the string, and in MULTILINE mode also matches before a newline.' Newlines are whitespace, and are removed before matching the regular expression, which means you can't write a transcript with a regular expression anchored to the end of each line of output.

Same story with the '^', which matches 'the start of the string, and in MULTILINE mode also matches immediately after each newline.' You can't write a transcript that checks for a regular expression at the beginning of each line of output.

Many regular expressions try and look for words in the output. Based on the documentation you would (rightly) think that your regular expressions should avoid the special sequence '\s' which checks for any whitespace character. However, you might not realize that you should also avoid '\w', the special sequence which matches Unicode word characters. You might use a regex like '\w+' intending to match a single word. When the code removes the whitespace, '\w+' will likely end up matching the entire result, across multiple lines of output. Probably not what you intended.

Given these many non-intuitive implications and behaviors, I recommend we change the behavior of the transcript testing feature to not do anything special with whitespace. This is a breaking change, and I don't recommend it lightly.

However, I'm a regular expression guy, and while trying to figure this all out, I wrote thirteen transcripts which included regular expressions. Only 2 of the thirteen worked as I expected, even when I had in mind the warning from the documentation to 'not include any whitespace expressions'.

If your subclass of Cmd2 has commands that return static and predictable results, then the current behavior probably suits you just fine. For any other usage, the current transcript behavior regarding regular expressions requires more patience than I believe we can rightfully expect from anyone.

[tleonhardt]

I know that personally about half the time I have tried using regexes with transcripts it didn't work properly and I've had to change or use a simpler regex and I didn't understand why. So anything we can do to make the tool easier to use and more intuitive would be great. I don't like making arbitrary backwards incompatible changes, but I'm fine with backwards incompatible changes that are made for a good reason.

So I think your recommendation is an excellent one.

[kotfu]

Sounds good. I'll write up the documentation for how this new approach would work, and maybe you can have a look see before I go write the code.

[tleonhardt]

I'd be happy to look over the documentation you write up. Thanks for tackling this!

[kotfu]

Here's my branch which now includes a first cut at the documentation for the revised approach: https://github.com/kotfu/cmd2/tree/fix/transcript_regexes

[tleonhardt]

@kotfu What you have in your branch looks great! It is a very significant improvement to the existing documentation and unit tests associated with transcript testing.

There are a few minor Sphinx-related bugs that doing a "make html" in the docs directory will reveal assuming you have Sphinx and the RTD Sphinx theme installed.

For example, in index.rst, the name in the toctree needs to match the name of the actual *.rst document. So now it says "transcription", but the file is called "transcript.rst".

But I think what you have is excellent both for the documentation itself and the refactored and expanded unit tests.

[kotfu]

Yeah, I didn't even run sphinx yet, just tried to hammer out the words. I'll get it all cleaned up as I continue working on it.