[cmd] Parse command exits with error in case of duplicated suppress c… #3253
Conversation
zomen2
added
WIP 💣
zomen2
force-pushed
the
fix_doubled_report_error
branch
2 times, most recently
from
4ca23fe to
c784e4a
Compare
| self._test_directory = os.path.dirname(os.path.abspath(inspect.getfile( | ||
| inspect.currentframe()))) |
There was a problem hiding this comment.
There is no need to use inspect module to get the current directory path. You can use the following code:
| self._test_directory = os.path.dirname(os.path.abspath(inspect.getfile( | |
| inspect.currentframe()))) | |
| self._test_directory = os.path.dirname(os.path.realpath(__file__)) |
| @@ -0,0 +1,10 @@ | |||
| 0c07579523063acece2d7aebd4357cac||suppress_export.cpp||foo1 simple||false_positive | |||
There was a problem hiding this comment.
I think this file is hardly connected to source file: https://github.com/zomen2/codechecker/blob/fix_doubled_report_error/analyzer/tests/projects/suppress/suppress_export.cpp
If we change the source file we need to change this file too. So it would be better to move this file back to the original location beside the source file.
There was a problem hiding this comment.
At this moment the test source does not allow this. A large refactor should be performed on function test code before your suggestion can be done.
| "--input-format", "plist", | ||
| "--verbose", "debug", test_plist_file] |
There was a problem hiding this comment.
| "--input-format", "plist", | |
| "--verbose", "debug", test_plist_file] | |
| test_plist_file] |
There was a problem hiding this comment.
I removed the last two switch. But the file format switch does not modify the behavior of the test, but gives more information.
| @@ -0,0 +1,6 @@ | |||
| void foo() | |||
There was a problem hiding this comment.
No there are two projects directories: in the analyzer part and in the web part. The module which handles the suppression comments can be found in the codechecker_common package: https://github.com/Ericsson/codechecker/blob/master/codechecker_common/source_code_comment_handler.py
For this reason I think we should create a common suppress project under this directory: https://github.com/Ericsson/codechecker/tree/master/codechecker_common/tests
So it would be codechecker_common/tests/projects/suppress. And we can move all the source files, suppress files, skip files under this project.
And test cases in the analyzer/web part can use this common project. This way we will test that the server and the parse command works the same way on the same code.
There was a problem hiding this comment.
See my comment above.
| // codechecker_suppress [deadcode.DeadStores] Same suppress comment twice. | ||
| // codechecker_suppress [deadcode.DeadStores] Same suppress comment twice. |
There was a problem hiding this comment.
This is not a typical use case when the type and the checker name is the same. I agree that we need to test it too but we need to introduce more test cases which test the followin scenarios too:
// codechecker_suppress [all] comment
// codechecker_suppress [deadcode.DeadStores] comment
// codechecker_suppress [all] comment
// codechecker_confirmed [deadcode.DeadStores] comment
// codechecker_suppress [all, deadcode.DeadStores] comment
For example on the second use case when I run the CodeChecker parse command I don't get an error message that multiple source code comments can be found for the same report but on the third use case I get this error. It should give an error in both cases.
Also now for every use cases we need to create a separte source files because it will give an error message on the first source code comment duplication. Maybe we can generate these source files on the fly. Simply creating a temp file, analyze and parse it. What do you think? We are doing the same for example at here: https://github.com/Ericsson/codechecker/blob/master/web/tests/functional/detection_status/test_detection_status.py#L82
There was a problem hiding this comment.
I fixed the implementation. New tests added to check these cases.
zomen2
force-pushed
the
fix_doubled_report_error
branch
from
c784e4a to
25e09a1
Compare
zomen2
force-pushed
the
fix_doubled_report_error
branch
3 times, most recently
from
5baae28 to
19cd926
Compare
zomen2
force-pushed
the
fix_doubled_report_error
branch
2 times, most recently
from
c31937c to
3fb90be
Compare
zomen2
force-pushed
the
fix_doubled_report_error
branch
from
3fb90be to
41c3ad9
Compare
…processing Functional tests enhanced to check semnthic of suppress comment language.
…ommands