Skip to content
This repository has been archived by the owner on Sep 1, 2023. It is now read-only.

Integrate Google Test and port all tests in testeverything #179

Merged
merged 62 commits into from
Oct 6, 2014

Conversation

utensil
Copy link
Member

@utensil utensil commented Sep 8, 2014

This PR is the work on #10 as per discussed at #10 (comment) (@rhyolight , @scottpurdy , @chetan51), using the Google Test framework . Replaces #150 .

This PR completed the following tasks:

  • generate JUnit-style report for nupic.tools to consume as the original C++ unit tests generate xUnit style reports #10 requests
  • porting original tests in testeverything with minimal modification
  • silent gcov so it will not generate long outputs to console
  • add tests for the re-implemented Tester

Decision TODOs:

  • decide a goal name for unit test, for now I'm using tests_unit
  • decide where the tests go, for now I've moved from src/test/testeverything to src/test/unit

Conflicts:
	.travis.yml
Regexes used:

TESTEQUAL\(([^\,]*"[^\,]*)\,([^\,\)]*)\)

TESTEQUAL\(([^\,]*str[^\,]*)\,([^\,\)]*)\)

TESTEQUAL\(([^\,]*)\,([^\,\)]*"[^\,\)]*)\)

TESTEQUAL\(([^\,]*)\,([^\,\)]*str[^\,\)]*)\)

TESTEQUAL2\(([^\,]*)\,([^\,]*"[^\,]*)\,([^\,\)]*)\)

TESTEQUAL2\(([^\,]*)\,([^\,]*str[^\,]*)\,([^\,\)]*)\)

TESTEQUAL2\(([^\,]*)\,([^\,]*)\,([^\,]*"[^\,]*)\)

TESTEQUAL2\(([^\,]*)\,([^\,]*)\,([^\,]*str[^\,]*)\)
@utensil
Copy link
Member Author

utensil commented Sep 21, 2014

@rhyolight @oxtopus The problem is solved at numenta/nupic-legacy#1299 (comment) . The problem is actually caused by:

  • I've placed gtest-all.cpp at $NUPIC_CORE/src/test/unit/, along with the tests, so when compiling unit tests in nupic.core, the CMake macro generate_executable(which assumes all the source file for a binary are in the same directory) will be happy
  • but $NUPIC/extensions/cpp_region/unittests/ also used Tester which in turn used gtest, then its linker will require gtest-all.cpp to be part of the source, or gtest has to be a library to be linked
  • and yet this requirement is not satisfied

I don't understand why it's not causing the same problem for linux, but this is the reason causing mac builds to fail.

My naive solution is to copy $NUPIC_CORE/src/test/unit/gtest-all.cpp to $NUPIC/extensions/cpp_region/unittests/, this is successful but somehow ugly. I need your input to tell what is a better solution:

  • keep this naive strategy
  • put gtest-all.cpp in $NUPIC_CORE/external/common, when building nupic.core, also build a gtest.a only for nupic.core and cpp_region unit tests to link
  • none of the above

@oxtopus
Copy link
Contributor

oxtopus commented Sep 21, 2014

Thanks for the detailed analysis! I prefer the second option.

@rhyolight
Copy link
Member

Nice job tracking this down, @utensil! Thanks a bunch. I was clueless.


Matt Taylor
OS Community Flag-Bearer
Numenta

On Sun, Sep 21, 2014 at 6:59 AM, Austin Marshall notifications@github.com
wrote:

Thanks for the detailed analysis! I prefer the second option.


Reply to this email directly or view it on GitHub
#179 (comment).

@utensil
Copy link
Member Author

utensil commented Oct 3, 2014

Will carry on with the second option.

@rhyolight
Copy link
Member

@utensil Let me know when you are ready and I will test again on OS X.

@utensil
Copy link
Member Author

utensil commented Oct 4, 2014

@rhyolight I think it's now ready both here and at numenta/nupic-legacy#1299

@utensil
Copy link
Member Author

utensil commented Oct 4, 2014

There's one weird thing: coverall is not commenting coverage report...

@utensil
Copy link
Member Author

utensil commented Oct 4, 2014

But I can see coverall functional at my branch:

image

@utensil
Copy link
Member Author

utensil commented Oct 4, 2014

The error output of nupic is:

Successfully installed python-coveralls requests sh
Cleaning up...
Traceback (most recent call last):
  File "/home/travis/build/numenta/nupic/bin/coveralls", line 9, in <module>
    load_entry_point('python-coveralls==2.4.2', 'console_scripts', 'coveralls')()
  File "/home/travis/build/numenta/nupic/local/lib/python2.7/site-packages/coveralls/__init__.py", line 73, in wear
    from coveralls.control import coveralls
  File "/home/travis/build/numenta/nupic/local/lib/python2.7/site-packages/coveralls/control.py", line 1, in <module>
    from coverage.control import coverage
ImportError: cannot import name coverage
Requirement already satisfied (use --upgrade to upgrade): python-coveralls in ./lib/python2.7/site-packages
Requirement already satisfied (use --upgrade to upgrade): PyYAML in ./lib/python2.7/site-packages (from python-coveralls)
Requirement already satisfied (use --upgrade to upgrade): requests in ./lib/python2.7/site-packages (from python-coveralls)
Requirement already satisfied (use --upgrade to upgrade): coverage in ./lib/python2.7/site-packages (from python-coveralls)
Requirement already satisfied (use --upgrade to upgrade): six in ./lib/python2.7/site-packages (from python-coveralls)
Requirement already satisfied (use --upgrade to upgrade): sh in ./lib/python2.7/site-packages (from python-coveralls)
Cleaning up...
Traceback (most recent call last):
  File "/home/travis/build/numenta/nupic/bin/coveralls", line 9, in <module>
    load_entry_point('python-coveralls==2.4.2', 'console_scripts', 'coveralls')()
  File "/home/travis/build/numenta/nupic/local/lib/python2.7/site-packages/coveralls/__init__.py", line 73, in wear
    from coveralls.control import coveralls
  File "/home/travis/build/numenta/nupic/local/lib/python2.7/site-packages/coveralls/control.py", line 1, in <module>
    from coverage.control import coverage
ImportError: cannot import name coverage
Done. Your build exited with 0.

This seems to be a separate issue brought in by other PRs. Will investigate and open a separate issue. This PR remains ready, @rhyolight .

@rhyolight
Copy link
Member

@utensil Great news! Ok @scottpurdy / @chetan51 / @oxtopus can one of you review this for the (hopefully) last time? Let's see if we can finally put this issue to bed today! 😄

@oxtopus
Copy link
Contributor

oxtopus commented Oct 6, 2014

Will do. Stand by.

oxtopus added a commit that referenced this pull request Oct 6, 2014
Integrate Google Test and port all tests in `testeverything`
@oxtopus oxtopus merged commit adb53f7 into numenta:master Oct 6, 2014
@rhyolight
Copy link
Member

💥 Thanks Austin!

@utensil utensil deleted the 10-google-test-rewrite branch October 7, 2014 03:50
@rhyolight rhyolight modified the milestone: Testing Oct 14, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants