Add option to include stack trace in debug() output #143
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #143 +/- ##
==========================================
- Coverage 96.29% 95.88% -0.42%
==========================================
Files 8 8
Lines 729 753 +24
Branches 111 115 +4
==========================================
+ Hits 702 722 +20
- Misses 21 23 +2
- Partials 6 8 +2
Continue to review full report in Codecov by Sentry.
|
devtools/debug.py
Outdated
|
|
||
| return DebugFrame(function, str(path), lineno) | ||
|
|
||
| def __init__(self, function: str, path: str, lineno: int): |
devtools/debug.py
Outdated
| prefix += f' ({self.warning})' | ||
| return f'{prefix}\n ' + '\n '.join(a.str(highlight) for a in self.arguments) | ||
|
|
||
| return prefix + '\n ' + '\n '.join(a.str(highlight) for a in self.arguments) |
There was a problem hiding this comment.
I don't think this needs to change.
There was a problem hiding this comment.
Thought it would be more efficient to avoid forcing python to evaluate the f-string, but not important.
devtools/debug.py
Outdated
| """ | ||
| BEWARE: this must be called from a function exactly `frame_depth` levels below the top of the stack. | ||
| """ | ||
| # HELP: any errors other than ValueError from _getframe? If so please submit an issue | ||
| try: | ||
| call_frame: 'FrameType' = sys._getframe(frame_depth) | ||
| except ValueError: | ||
| # "If [ValueError] is deeper than the call stack, ValueError is raised" | ||
| # "If [the given frame depth] is deeper than the call stack, |
There was a problem hiding this comment.
Happy to undo this, but this comment made no sense to me. I had to go find that quote in the actual python docs to figure out what it meant.
| @@ -225,4 +262,18 @@ def _process_args(self, ex: 'Any', args: 'Any', kwargs: 'Any') -> 'Generator[Deb | |||
| yield self.output_class.arg_class(value, name=name, variable=kw_arg_names.get(name)) | |||
|
|
|||
|
|
|||
| def _make_call_context(call_frame: 'Optional[FrameType]', trace: bool) -> 'List[DebugFrame]': | |||
There was a problem hiding this comment.
might make most sense for this to be part of DebugFrame
There was a problem hiding this comment.
There's not really much difference between a function and a static method, but all else being equal, I prefer functions. I think they're easier to understand because you know they only have access to the public APIs of any classes they use. But I can make it a method if you care strongly about it.
devtools/debug.py
Outdated
| @@ -22,6 +22,47 @@ | |||
| StrType = str | |||
|
|
|||
|
|
|||
| class DebugFrame: | |||
There was a problem hiding this comment.
probably best to put this in a new module, otherwise it should go at the bottom of debug.py.
There was a problem hiding this comment.
I think this class is pretty tightly-coupled to all the other classes in debug.py, so I just moved it to the bottom.
requirements/testing.in
Outdated
| pytest-tmp-files | ||
| parametrize-from-file |
There was a problem hiding this comment.
I don't think we need these extra dependencies, it should be a few lines of code to do this manually, either with strings in the python code, or reading files from a new directory in tests.
There was a problem hiding this comment.
you can probably use python files with a multiline string at the end of the for expected stdout.
There was a problem hiding this comment.
I rewrote the tests using vanilla python.
|
Here's a screenshot: |
As discussed in #105, this PR adds some options to have
debug()output stack traces.While I was working on this, I realized that there are two reasonable way to format stack traces. The first is to match the existing style that
debug()uses:The second is to match the style that python itself uses:
Both styles have the same information, but the python style is more verbose and includes the actual text of the line in question. I decided to use the devtools style, because it "fits" better and I didn't think the extra verbosity would be that helpful for the kinds of things I envision debugging with this feature, but I'm not confident in this decision. I might change my mind after living with it for a while. If anyone else has an opinion on which style to use, I'd be happy to hear it.
Another aspect of this PR that I should draw attention to is the new unit tests. I think that all the stack trace tests need to happen in subprocesses, because otherwise the stack traces would include ≈20 pytest frames (which would be likely to change between different versions of pytest). To avoid duplicating the boilerplate required to run a test in a subprocess, I decided to write these tests using two of my own libraries:
parametrize_from_fileandpytest_tmp_files. The specific code I wrote here is very similar to this example from the documentation. I think that I wrote these tests in the most understandable, maintainable way, but I'd understand if you were skeptical about adding two relatively unknown dependencies to your project. If so, let me know and I'd be happy to rewrite the tests using vanilla pytest. (Also, I had to regenerate therequirements/testing.txtfile, and I'm not sure I did that in the right way.)