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

Try to integrate googletest as unit-test. #34

Closed
wants to merge 3 commits into from

Conversation

chhsiao1981
Copy link
Contributor

Try to integrate googletest as unit-test.

@wens
Copy link
Contributor

wens commented Dec 11, 2017

I don't like submodules. And I don't like vendoring.

What's wrong with using packages provided by the OS?

@chhsiao1981
Copy link
Contributor Author

chhsiao1981 commented Dec 11, 2017

I am not very familiar with googletest, but the official website seems to recommend compiling googletest as part of source code in the test part.

The other possibility is to not use googletest and use other framework instead.

https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#why-is-it-not-recommended-to-install-a-pre-compiled-copy-of-google-test-for-example-into-usrlocal

@chhsiao1981
Copy link
Contributor Author

Since pttbbs is written in pure c,

it's possible that we just switch to check (looks like the project is still update-to-date):

https://libcheck.github.io/check/index.html

@robertabcd
Copy link
Contributor

I hope to see real unit tests written before choosing the framework.
Google test is usually in libgtest-dev if you are on Debian/Ubuntu. I prefer this over submodules. Also, try not to use two build systems.

@chhsiao1981 chhsiao1981 force-pushed the hsiao.googletest branch 2 times, most recently from b02597a to 4492850 Compare December 12, 2017 10:17
@chhsiao1981
Copy link
Contributor Author

  1. I modified the include for googletest as from libgtest-dev. There should be no vendoring issue now~
    (I try to use cmake -DGTEST_DIR=/usr/src/googletest to solve the gtest-compiling issue~
    Hopefully this approach can be cross-platform adaptable~)

  2. About the make / cmake thing, I am aware of this issue,
    but the purpose of this pr is to have some unit-test available for the project first,
    and then we can start to do some refactoring more stably.

    We can always improve the make / cmake thing in the future.

  3. In terms of "real unit-test", I'll apply this unit-test to the refactoring soon.

https://github.com/chhsiao1981/pttbbs/tree/hsiao.googletest/tests

@chhsiao1981
Copy link
Contributor Author

Intended to use Makefile (pmake) instead of cmake.

For now:

  1. Need to separate mbbsd/Makefile with mbbsd/mbbsdvars.mk
  2. Need to ignore many .o in mbbsd due to tangling with mbbsd.o, and there is "main" in mbbsd.o
    conflicting "main" in test.

@chhsiao1981 chhsiao1981 changed the title 1. git submodule googletest. 2. tests Try to integrate googletest as unit-test. Dec 28, 2017
@chhsiao1981
Copy link
Contributor Author

hi @robertabcd

I revised this PR and provided some basic examples as the templates for the unittest.
(tests/test_common/test_sys/test_string.cc,
tests/test_mbbsd/test_bbs.cc)

I added tests/test_mbbsd/mbbsd_mock.c as mock for mbbsd.c because there is main in mbbsd.c
The only difference between mbbsd_mock.c and mbbsd.c is that main becomes mock_main.

Hopefully the revision satisfies the PR and others can contribute more on unittest.

@chhsiao1981
Copy link
Contributor Author

I'll close this for now and revise the pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants