-
Notifications
You must be signed in to change notification settings - Fork 9
Embedded Python Coverage Support #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ts in a python list, started and stopped along with the LineByLine Monitor
…t it in the CodeUnit
…tores the MethodMap (line numbers for each method), and executable lines got a bug fix. UpdateSourceMap now saves mapped line numbers from .py to .cls
…PCJG (as well as code that unsuccessfully tries to save it into SQL)
…then wrote StorePyCoverage to store that data in a Coverage
Added ..CoveragePythons Changed AddCoverageClass to add to ..CoveragePythons Added ..PyCoverageTargets Changed SetCoverageTargets (which is called when the user actually passes in coverage classes and routines, instead of it being in coverage.list) to fill in PyCoverageTargets with just the non .py part of the class name Got UpdateCoverageTargetsForTestDirectory to update ..PyCoverageTargets in the case where there’s a coverage.list Handle StartCoverageTracking seems redundant to include the python in the new / relevant stuff: it’s new or relevant if its class is new / relevant Added the python components of the old relevant classes into tPyRelevantTargets With that being said, I think we need Snapshot to return the python targets for the relevant classes In Snapshot: checked if the class has a python part (if it does, it’s relevant), and added those to the list of python new relevant targets. Took a snapshot of each of those Combined tPyNewRelevantTargets into tPyRelevantTargets
…erageTracking to StoreIntRoutine to store the coverage results from the .py CodeUnits, resulting in python lines being marked as covered in .cls files
…e python now working end to end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in weekly touchpoint today:
- Parallelize taking the snapshot of the Python routines and fetching the executable lines back to the .cls files -> no need for this
- Do we want to store the Python trace data differently (right now it’s just in the global ^IRIS.TEMPCJG) -- if so, need to fix the crashing error that I never resolved - this is a fine approach, just use a better global name
- What is cyclomatic complexity, and what are we using it for? Will need to implement for python CodeUnits probably - using radon with requirements.txt and IPM's support for python dependency installation should work
- Right now, CodeUnit’s LineToMethodMap property (already existed) and the CoveragePythons / PyCoverageTarget properties in Manager (I added) aren’t doing anything. Keep or get rid of them? - LineToMethodMap is needed for .INT/.CLS, let's keep all for consistency
- Manually change some of the lines to not executable: for instance, it seems weird that the class method definition gets marked as executed even if the class method is never called (but sys.settrace does show it as executed) - only really matters for class method definition, solution is to not mark as executable
- Write unit tests for the new python code - great idea!
- Use of $Namespace vs. not in different contexts - good question, see #13 (which is pretty useless)
- requirements.txt example - see Anya's work
…into EmbeddedPython Adding changes from release 3.1.0
…re correct (per radon)
Turns out there's no .INT routine generated for .CLS files with only embedded python, which means that we can no longer piggyback finding out which python files to track by using the .CLS files. I now keep track of relevant and new python coverage targets separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have relatively few comments and most of them are more stylistic than a matter of correctness - very well done!!
CHANGELOG.md
Outdated
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. | |||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | |||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|||
## [3.2.0] - 2024-07-08 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should say Unreleased until released
…amplecovlist.list in the first directory now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed today, we can look into aligning the start of python methods in subsequent PRs.
Turns out to be a non-issue :) |
…Coverage into EmbeddedPython not sure what caused the difference?
…is a python line or not
…into EmbeddedPython fetching changes from PR 41
…starts and finishes
Added in the listener interface from PR #42
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go - just need to clean up CHANGELOG.md to reflect all under 3.2.0 (including listeners which got merged here, fine if that goes in with this PR too to save some time)
Coverage tracking results now include embedded python methods in .CLS files
Implements #29