[issue9315] The trace module lacks unit tests

[Alexander Belopolsky]

Alexander Belopolsky <belopolsky at users.sourceforge.net> added the comment:

Comments on issue9315.27-maint.5.patch:

1. I think you forgot to svn add Lib/test/__init__.py.
2. Instead of "import tracedmodules.testmod", please use something like "from test.tracedmodules import testmod".  There is no need to use implicit relative import and this is different from accepted practice.

3. 

def traced_func_importing(aa, bb):
    from tracedmodules.testmod import func

Same comment as in #2 above.  No need for implicit relative import here.  Also I am not sure it is interesting to test tracing the import statement inside a function.  Tests for tracing calls to imported functions are good, but I am not sure import itself generates any interesting events.  Just give it a thought - I don't have an informed opinion.

4. Shouldn't all traced functions go to tracedmodules?

5. Please add comments to functions used for line tracing that changing relative or absolute (if matters) line numbers will break the tests.

6. Add comments to testmod.py.

7. You lost changes I made in issue9315.4-release27.patch. Specifically, using trailing underscores or double letters to resolve conflicts between variable names is not common style.  Trailing underscore convention is for resolving conflicts with python keywords.

8. Please rewrap text in README to fit in 80 columns.  In fact, this text belongs to __init__.py docstring and the comment about importance of function location should go next to each (currently one) function for which location is important.

9. fix_pyc_ext(filename) description is misleading.  It does not care about incoming filename extension and just whatever extension with '.py'.  This is probably good because it works with both '.pyc' and '.pyo', but the name and the docstring suggest otherwise. Note the similar logic in the trace module itself is implemented as follows:

            if filename.endswith((".pyc", ".pyo")):
                filename = filename[:-1]   

I also feel that three functions to just compute ('test_trace.py', 'test_trace') tuple is an overkill.  Please look in the inspect module for possible alternatives.  Also rather than recomputing these strings in each test case, I would just assign them to module global variables say THIS_FILE_NAME and THIS_MODULE_NAME. 

10. A nitpick.  I don't think I've ever seen test_main() function called "Driver" in the python test suite.  Please try to keep consistency in terminology and coding style between the test modules to the extent it is practical.

11.  Similar to #10.  I've changed 'ZZZ' to 'XXX' in issue9315.4-release27.patch, but you lost that change.  See msg112230 above.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue9315>
_______________________________________