Python: Port test model to API graphs #9368
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With no virtual environment enabled, none of the third-party library test case are found.
Notice that although we loose the contrived examples in `test.py`, we do gain support for real-world test-case construction, which seems worth the tradeoff.
tausbn
requested changes
May 30, 2022
Comment on lines
-1
to
-3
| | test.py:4:1:4:23 | Class MyTest | | ||
| | test.py:6:5:6:21 | Function test_1 | | ||
| | test.py:9:5:9:21 | Function test_2 | |
There was a problem hiding this comment.
Is the fact that these are no longer matched intended? If so, then maybe we should just get rid of test.py entirely?
There was a problem hiding this comment.
yeah I noted in a8b4b6a:
Notice that although we loose the contrived examples in
test.py, we do
gain support for real-world test-case construction, which seems worth
the tradeoff.
I think I'll delete that file 👍
There was a problem hiding this comment.
Ah! I didn't see that commit message. Thanks!
tausbn
approved these changes
May 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found an old evaluation on LGTM for comparing the difference. Overall it looks good 👍
One improvement is that the old modeling would also flag custom test runners (subclasses of
unittest.TextTestRunner) and test loaders (subclasses ofunittest.TestLoader). Although these are related to testing, they are certainly not a unit test class ;)The new modeling captures quite a lot of new real testcases, from subclasses such as
from flask_testing import TestCase, LiveServerTestCasefrom nose.plugins import doctests as npd; npd.DocTestCasetornado.testing.AsyncHTTPTestCasefrom django.test import TestCaseOverall I think this looks like an improvement in behavior for our test-detection, although it isn't used for as much with Code Scanning as it was for LGTM. It also moves us away from points-to. Seems like a win-win to me 😊