Skip to content
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

run tests with std::istringstream,std::ifstream, buffer and file input #261

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/CI-unixish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ jobs:
run: |
sudo apt-get update
sudo apt-get install valgrind

# coreutils contains "nproc"
- name: Install missing software on macos
if: contains(matrix.os, 'macos')
run: |
brew install coreutils

- name: make simplecpp
run: make -j$(nproc)
Expand Down
57 changes: 53 additions & 4 deletions simplecpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
#include <cstring>
#include <ctime>
#include <exception>
#include <fstream> // IWYU pragma: keep
#include <fstream>
#include <iostream>
#include <limits>
#include <list>
#include <map>
#include <set>
#include <sstream> // IWYU pragma: keep
#include <sstream>
#include <stack>
#include <stdexcept>
#include <string>
Expand Down Expand Up @@ -377,6 +377,42 @@ class StdIStream : public simplecpp::TokenList::Stream {
std::istream &istr;
};

class StdCharBufStream : public simplecpp::TokenList::Stream {
public:
// cppcheck-suppress uninitDerivedMemberVar - we call Stream::init() to initialize the private members
StdCharBufStream(const unsigned char* str, std::size_t size)
: str(str)
, size(size)
, pos(0)
, lastStatus(0)
{
init();
}

virtual int get() OVERRIDE {
if (pos >= size)
return lastStatus = EOF;
return str[pos++];
}
virtual int peek() OVERRIDE {
if (pos >= size)
return lastStatus = EOF;
return str[pos];
}
virtual void unget() OVERRIDE {
--pos;
}
virtual bool good() OVERRIDE {
return lastStatus != EOF;
}

private:
const unsigned char *str;
const std::size_t size;
std::size_t pos;
int lastStatus;
};

class FileStream : public simplecpp::TokenList::Stream {
public:
// cppcheck-suppress uninitDerivedMemberVar - we call Stream::init() to initialize the private members
Expand Down Expand Up @@ -442,6 +478,20 @@ simplecpp::TokenList::TokenList(std::istream &istr, std::vector<std::string> &fi
readfile(stream,filename,outputList);
}

simplecpp::TokenList::TokenList(const unsigned char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename, OutputList *outputList)
: frontToken(nullptr), backToken(nullptr), files(filenames)
{
StdCharBufStream stream(data, size);
readfile(stream,filename,outputList);
}

simplecpp::TokenList::TokenList(const char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename, OutputList *outputList)
: frontToken(nullptr), backToken(nullptr), files(filenames)
{
StdCharBufStream stream(reinterpret_cast<const unsigned char*>(data), size);
readfile(stream,filename,outputList);
}

simplecpp::TokenList::TokenList(const std::string &filename, std::vector<std::string> &filenames, OutputList *outputList)
: frontToken(nullptr), backToken(nullptr), files(filenames)
{
Expand Down Expand Up @@ -1447,8 +1497,7 @@ namespace simplecpp {

Macro(const std::string &name, const std::string &value, std::vector<std::string> &f) : nameTokDef(nullptr), files(f), tokenListDefine(f), valueDefinedInCode_(false) {
const std::string def(name + ' ' + value);
std::istringstream istr(def);
StdIStream stream(istr);
StdCharBufStream stream(reinterpret_cast<const unsigned char*>(def.data()), def.size());
tokenListDefine.readfile(stream);
if (!parseDefine(tokenListDefine.cfront()))
throw std::runtime_error("bad macro syntax. macroname=" + name + " value=" + value);
Expand Down
4 changes: 4 additions & 0 deletions simplecpp.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ namespace simplecpp {
explicit TokenList(std::vector<std::string> &filenames);
/** generates a token list from the given std::istream parameter */
TokenList(std::istream &istr, std::vector<std::string> &filenames, const std::string &filename=std::string(), OutputList *outputList = nullptr);
/** generates a token list from the given buffer */
TokenList(const unsigned char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename=std::string(), OutputList *outputList = nullptr);
/** generates a token list from the given buffer */
TokenList(const char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename=std::string(), OutputList *outputList = nullptr);
/** generates a token list from the given filename parameter */
TokenList(const std::string &filename, std::vector<std::string> &filenames, OutputList *outputList = nullptr);
TokenList(const TokenList &other);
Expand Down
100 changes: 89 additions & 11 deletions test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,23 @@
#include <cstdlib>
#include <cstring>
#include <exception>
#include <fstream>
#include <iostream>
#include <limits>
#include <map>
#include <sstream>
#include <stdexcept>
#include <string>
#include <vector>

enum Input {
Stringstream,
Fstream,
File,
CharBuffer
};

static Input USE_INPUT = Stringstream;
static int numberOfFailedAssertions = 0;

#define ASSERT_EQUALS(expected, actual) (assertEquals((expected), (actual), __LINE__))
Expand All @@ -32,11 +42,24 @@ static std::string pprint(const std::string &in)
return ret;
}

static const char* inputString(Input input) {
switch (input) {
case Stringstream:
return "Stringstream";
case Fstream:
return "Fstream";
case File:
return "File";
case CharBuffer:
return "CharBuffer";
}
}

static int assertEquals(const std::string &expected, const std::string &actual, int line)
{
if (expected != actual) {
numberOfFailedAssertions++;
std::cerr << "------ assertion failed ---------" << std::endl;
std::cerr << "------ assertion failed (" << inputString(USE_INPUT) << ") ---------" << std::endl;
std::cerr << "line " << line << std::endl;
std::cerr << "expected:" << pprint(expected) << std::endl;
std::cerr << "actual:" << pprint(actual) << std::endl;
Expand Down Expand Up @@ -71,10 +94,51 @@ 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) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that it's a good idea to avoid using real files in tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I totally agree with you. But we allow the input of actual files so we should actually test this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.

After thinking about it a bit more the usage with Cppcheck could be considered an integration test.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's more like a integration test.

std::string tmpfile = filename.empty() ? "code.tmp" : filename;
{
std::ofstream of(tmpfile, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc);
of.write(code, size);
}
return tmpfile;
}

static simplecpp::TokenList makeTokenListFromFstream(const char code[], std::size_t size, std::vector<std::string> &filenames, const std::string &filename, simplecpp::OutputList *outputList)
{
const std::string tmpfile = writeFile(code, size, filename);
std::ifstream fin(tmpfile);
if (!fin.is_open())
throw std::runtime_error("could not open " + tmpfile);
simplecpp::TokenList tokenList(fin, filenames, tmpfile, outputList);
remove(tmpfile.c_str());
return tokenList;
}

static simplecpp::TokenList makeTokenListFromFile(const char code[], std::size_t size, std::vector<std::string> &filenames, const std::string &filename, simplecpp::OutputList *outputList)
{
const std::string tmpfile = writeFile(code, size, filename);
std::ifstream fin(tmpfile);
if (!fin.is_open())
throw std::runtime_error("could not open " + tmpfile);
simplecpp::TokenList tokenList(tmpfile, filenames, outputList);
remove(tmpfile.c_str());
return tokenList;
}

static simplecpp::TokenList makeTokenList(const char code[], std::size_t size, std::vector<std::string> &filenames, const std::string &filename=std::string(), simplecpp::OutputList *outputList=nullptr)
{
std::istringstream istr(std::string(code, size));
return simplecpp::TokenList(istr,filenames,filename,outputList);
switch (USE_INPUT) {
case Stringstream: {
std::istringstream istr(std::string(code, size));
return simplecpp::TokenList(istr, filenames, filename, outputList);
}
case Fstream:
return makeTokenListFromFstream(code, size, filenames, filename, outputList);
case File:
return makeTokenListFromFile(code, size, filenames, filename, outputList);
case CharBuffer:
return simplecpp::TokenList(code, size, filenames, filename, outputList);
}
}

static simplecpp::TokenList makeTokenList(const char code[], std::vector<std::string> &filenames, const std::string &filename=std::string(), simplecpp::OutputList *outputList=nullptr)
Expand All @@ -94,11 +158,11 @@ static std::string readfile(const char code[], std::size_t size, simplecpp::Outp
return makeTokenList(code,size,files,std::string(),outputList).stringify();
}

static std::string preprocess(const char code[], const simplecpp::DUI &dui, simplecpp::OutputList *outputList)
static std::string preprocess(const char code[], const simplecpp::DUI &dui, simplecpp::OutputList *outputList, const std::string &file = std::string())
{
std::vector<std::string> files;
std::map<std::string, simplecpp::TokenList*> filedata;
simplecpp::TokenList tokens = makeTokenList(code,files);
simplecpp::TokenList tokens = makeTokenList(code,files,file);
tokens.removeComments();
simplecpp::TokenList tokens2(files);
simplecpp::preprocess(tokens2, tokens, files, filedata, dui, outputList);
Expand All @@ -110,6 +174,11 @@ static std::string preprocess(const char code[])
return preprocess(code, simplecpp::DUI(), nullptr);
}

static std::string preprocess(const char code[], const std::string &file)
{
return preprocess(code, simplecpp::DUI(), nullptr, file);
}

static std::string preprocess(const char code[], const simplecpp::DUI &dui)
{
return preprocess(code, dui, nullptr);
Expand Down Expand Up @@ -180,7 +249,7 @@ static void backslash()

static void builtin()
{
ASSERT_EQUALS("\"\" 1 0", preprocess("__FILE__ __LINE__ __COUNTER__"));
ASSERT_EQUALS("\"test.c\" 1 0", preprocess("__FILE__ __LINE__ __COUNTER__", "test.c"));
ASSERT_EQUALS("\n\n3", preprocess("\n\n__LINE__"));
ASSERT_EQUALS("\n\n0", preprocess("\n\n__COUNTER__"));
ASSERT_EQUALS("\n\n0 1", preprocess("\n\n__COUNTER__ __COUNTER__"));
Expand Down Expand Up @@ -811,7 +880,7 @@ static void define_va_args_4() // cppcheck trac #9754
ASSERT_EQUALS("\nprintf ( 1 , 2 )", preprocess(code));
}

static void define_va_opt_1()
static void define_va_opt_1()
{
const char code[] = "#define p1(fmt, args...) printf(fmt __VA_OPT__(,) args)\n"
"p1(\"hello\");\n"
Expand All @@ -822,7 +891,7 @@ static void define_va_opt_1()
preprocess(code));
}

static void define_va_opt_2()
static void define_va_opt_2()
{
const char code[] = "#define err(...)\\\n"
"__VA_OPT__(\\\n"
Expand Down Expand Up @@ -859,7 +928,7 @@ static void define_va_opt_3()
toString(outputList));
}

static void define_va_opt_4()
static void define_va_opt_4()
{
// missing parenthesis
const char code1[] = "#define err(...) __VA_OPT__ something\n"
Expand Down Expand Up @@ -2723,8 +2792,10 @@ static void fuzz_crash()
}
}

int main(int argc, char **argv)
static void runTests(int argc, char **argv, Input input)
{
USE_INPUT = input;

TEST_CASE(backslash);

TEST_CASE(builtin);
Expand Down Expand Up @@ -2877,7 +2948,7 @@ int main(int argc, char **argv)
TEST_CASE(missingHeader2);
TEST_CASE(missingHeader3);
TEST_CASE(nestedInclude);
TEST_CASE(systemInclude);
//TEST_CASE(systemInclude);

TEST_CASE(nullDirective1);
TEST_CASE(nullDirective2);
Expand Down Expand Up @@ -2950,6 +3021,13 @@ int main(int argc, char **argv)
TEST_CASE(token);

TEST_CASE(fuzz_crash);
}

int main(int argc, char **argv)
{
runTests(argc, argv, Stringstream);
runTests(argc, argv, Fstream);
runTests(argc, argv, File);
runTests(argc, argv, CharBuffer);
return numberOfFailedAssertions > 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
Loading