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

Setup unit test framework without the need to include source files #82

Closed
dschlaep opened this issue Nov 21, 2017 · 2 comments
Closed
Labels
Milestone

Comments

@dschlaep
Copy link
Member

Currently, unit tests only compile if the test/*.cc files #include "SW_*.c" the relevant SOILWAT2 source files.

This is obviously bad. We need to set up our unit test framework without the need to include source files

@dschlaep dschlaep added the bug label Nov 21, 2017
@dschlaep dschlaep added this to the Code testing milestone Nov 21, 2017
dschlaep added a commit that referenced this issue Nov 21, 2017
- addressing issue #82
- Remove source files from test/*.cc unit test files
- link SOILWAT2-library to 'test' target of make

- however, this does not compile and errors out with
> Undefined symbols for architecture x86_64:
  "RandBeta(float, float)", referenced from:
      (anonymous
namespace)::BetaGeneratorTest_ZeroToOneOutput_Test::TestBody() in
test_rands-0f5d64.o
      (anonymous
namespace)::BetaGeneratorDeathTest_Errors_Test::TestBody() in
test_rands-0f5d64.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see
invocation)
@dschlaep
Copy link
Member Author

g++ -isystem googletest/googletest/include -pthread libgtest.a libSOILWAT2.a test/*.cc
-o sw_test
Undefined symbols for architecture x86_64:
"RandBeta(float, float)", referenced from:
(anonymous namespace)::BetaGeneratorTest_ZeroToOneOutput_Test::TestBody() in test_rands-6fe717.o
(anonymous namespace)::BetaGeneratorDeathTest_Errors_Test::TestBody() in test_rands-6fe717.o
ld: symbol(s) not found for architecture x86_64

I don't understand why I get undefined symbols for architecture x86_64 errors...

Maybe the problem is that I compile libSOILWAT2.a with gcc and here I have to use g++ to compile with the googletest because it is c++?

  • However, if I don't include libSOILWAT2.a but instead the c-source files $(sources) directly (e.g., following https://stackoverflow.com/questions/41453253/googletest-cmake-undefined-symbols-for-architecture-x86-64) -- each prefixed with -x c --, then I get exactly the same error Undefined symbols for architecture x86_64.
  • If I don't include the c-source files $(sources) as c-language files, but instead as c++-files, then I get tons of additional errors such as error: cannot initialize return object of type 'OutPeriod' with an lvalue of type 'IntUS' (aka 'unsigned short') etc.

dschlaep added a commit that referenced this issue Nov 21, 2017
- defined CC, CFLAGS, CXX, CXXFLAGS, and LDIBS
- specified target architecture (as this was mentioned online that
different steps may target different architectures and that this may be
the problem underlying issue #82 -- but it didn't help)
- initialize global variable in main() of unit tests
- realized that linking with the libmath also works on macOS if done
correctly --> removed target 'binl'
dschlaep added a commit that referenced this issue Nov 21, 2017
- bad practice, see issue #82 which remains unsolved
- I want this branch to be merged into master so that I can use the
last set of commits as basis for development in other branches. sorry.
dschlaep added a commit that referenced this issue Nov 21, 2017
- removed compiler flag '-arch' because it didn't help addressing issue
#82
dschlaep added a commit that referenced this issue Nov 22, 2017
- avoid compiler warnings "warning: parameter set but not used"
- remove explicit definition of CC and CXX (e.g., so that CIs can set
their own, e.g., on travis we want to test with both clang and gnu gcc)
- add SOILWAT2 library to LDLIBS instead (now needs LDFLAGS)
- still does not resolve issue #82

- apparently, compilers on my machine produce different warnings/errors
than travis-ci and appveyor-ci (I can compile without warnings/errors
while they produce warnings and error out)
@dschlaep
Copy link
Member Author

When I include the SOILWAT2 source files as library to link to my unit test executable, it fails with undefined symbols ...:

c++ -Wall -Wextra -std=gnu++11 -L. -isystem googletest/googletest/include -pthread test/*.cc -o sw_test -lgtest -lSOILWAT2 -lm			

When I list the SOILWAT2 source files directly in the compile command, it works:

c++ -Wall -Wextra -std=gnu++11 -L. -isystem googletest/googletest/include -pthread rands.c generic.c mymemory.c filefuncs.c test/*.cc -o sw_test -lgtest

It produces warnings clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated. When I include the -x c flag to indicate c-code files, then I get the same undefined symbols error. It seems that the CXX linker can only successfully find symbols when they are compiled as c++; it fails when they are interpreted as c code (with -x c) or when they are linked as library (that was compiled with a c-compiler).

So I should compile a c++ version of libSOILWAT2. The problem is that we have a good portion of ancient, weird code that is not compatible with c++, e.g., all they typedef enum {...} Months; and then Months m; for (m = Jan; m <= Dec; m++) constructs.

dschlaep added a commit that referenced this issue Nov 22, 2017
- now close #82

We still cannot include SW_Output.c when unit testing:
* "error: cannot increment expression of enum type" (e.g., OutKey,
OutPeriod)
* "error: assigning to 'OutKey' from incompatible type 'int'", and
similar cases

--> for now, exclude SW_Output.c (see new issue #85), work on fixing
the errors under c++ and then

- address c++ compile "warning: ISO C++11 does not allow conversion
from string literal to 'char *'"
--> don't define such variables as "char *" but instead as "char const
*" (see
https://stackoverflow.com/questions/20944784/why-is-conversion-from-stri
ng-constant-to-char-valid-in-c-but-invalid-in-c#20944858)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant