test.cpp: also run tests with char buffer #261
Conversation
firewave
changed the title
firewave
changed the title
std::istringstream and std::ifstream
test.cpp
Outdated
|
|
||
| #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
force-pushed
the
test-x
branch
2 times, most recently
from
5d99779 to
b196f5b
Compare
firewave
changed the title
std::istringstream,std::ifstream,std::string and file inputstd::istringstream,std::ifstream, buffer and file input
|
This could be implemented in the Python tests instead by making this modes available via |
|
Thinking about it I think this is still valid except for the file input. |
firewave
force-pushed
the
test-x
branch
4 times, most recently
from
929222a to
07a2beb
Compare
firewave
changed the title
std::istringstream,std::ifstream, buffer and file input
|
I removed everything which generates a file. I will add flags to the CLI to be able to use those in the Python tests but that won't have the same coverage as using it in the unit tests and might detect some issues only as in drive-by way (as experienced in #566). |
firewave
force-pushed
the
test-x
branch
2 times, most recently
from
447bb72 to
90e5701
Compare
2 participants
No description provided.