run tests with std::istringstream,std::ifstream, buffer and file input
#261
Conversation
firewave
changed the title
firewave
changed the title
std::istringstream and std::ifstream
| @@ -77,17 +93,39 @@ static void testcase(const std::string &name, void (*f)(), int argc, char * cons | |||
|
|
|||
| #define TEST_CASE(F) (testcase(#F, F, argc, argv)) | |||
|
|
|||
| static std::string writeFile(const char code[], std::size_t size, const std::string &filename) { | |||
There was a problem hiding this comment.
I feel that it's a good idea to avoid using real files in tests.
There was a problem hiding this comment.
In general I totally agree with you. But we allow the input of actual files so we should actually test this.
There was a problem hiding this comment.
I still feel skeptic. if we assume that the stream input is bug free then the test should work the same with a string input or a file. if we extrapolate this then this should be done in cppcheck also. I fear I don't like this.
There was a problem hiding this comment.
Sorry for the late reply.
Keeping tests simple is great but I think there should be proper coverage and historically that is something we don't have in either Cppcheck or simplecpp. 😐
Also all the include files are always being read from the disk and not from memory. Not having any tests which rely on temporary files indicates that there is no coverage for reading includes at all. I have not looked into this yet.
So it seems instead of avoiding tests with files on disk we actually need to add much more.
There was a problem hiding this comment.
Having some system test that uses files is good but we do have some such system tests. Testing that APIs work as they should is out of scope.
There was a problem hiding this comment.
Without this I am not able to test the changes in #244. If that is fine with you I will try to finish up that PR without any unit tests.
I will prepare another PR to highlight with more tests to highlight the underlying testing issue.
There was a problem hiding this comment.
If that is fine with you I will try to finish up that PR without any unit tests.
yes I would prefer that in this case.
There was a problem hiding this comment.
Great.
After thinking about it a bit more the usage with Cppcheck could be considered an integration test.
There was a problem hiding this comment.
yes it's more like a integration test.
firewave
changed the title
std::istringstream and std::ifstreamstd::istringstream,std::ifstream and file input
firewave
changed the title
std::istringstream,std::ifstream and file inputstd::istringstream,std::ifstream,std::string and file input
firewave
changed the title
std::istringstream,std::ifstream,std::string and file inputstd::istringstream,std::ifstream, buffer and file input
No description provided.