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

Run C++ nupic.core tests through new gtest interface #1299

Closed
wants to merge 14 commits into from

Conversation

utensil
Copy link
Member

@utensil utensil commented Sep 13, 2014

Fixes #1298 .

This PR is NOT ready to merge until:

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 3cf7b44 on utensil:1298-gtest into 1461a5a on numenta:master.

NUPIC_CORE_REMOTE = 'git://github.com/numenta/nupic.core.git'
NUPIC_CORE_COMMITISH = '6b6afc1ef03dbfc2a183bacc6dfa85f5e758260a'
NUPIC_CORE_REMOTE = 'git://github.com/utensil/nupic.core.git'
NUPIC_CORE_COMMITISH = '7daa677c0a1b1296066ab2380dbe1e03b8c568cb'
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming this commit sha is in numenta/nupic.core, the line above needs to be reverted to point to the right github fork.

Copy link
Member

Choose a reason for hiding this comment

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

I think @utensil is aware of it, he mentions this in TODO.

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR shouldn't require a reference to an outside fork in .nupic_modules, even if meant to be temporary. If these PRs need to be coordinated, let's get the nupic.core changes pushed to a branch on numenta/nupic.core so that at least the SHA is valid (even if not on master).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is similar to the situation of the pip-installable PR. But adding a branch to numenta:nupic.core can only be done by a owner.

@rhyolight rhyolight added this to the Testing milestone Sep 15, 2014
@breznak
Copy link
Member

breznak commented Sep 18, 2014

@utensil @rhyolight @oxtopus let's re-trigger build here to see OSX , regarding the discussion in numenta/nupic.core-legacy#179

Merge numenta:master@HEAD
@utensil
Copy link
Member Author

utensil commented Sep 18, 2014

Triggered. Let's see what happens.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6387751 on utensil:1298-gtest into d466667 on numenta:master.

@utensil
Copy link
Member Author

utensil commented Sep 19, 2014

image

@rhyolight
Copy link
Member

@utensil
Copy link
Member Author

utensil commented Sep 21, 2014

Updating the SHA1 caused this PR to be affected by the transformation to C++ 11, which is in progress and failing at #1264 . This is why the latest build is dark on all OS and compilers.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 39804e1 on utensil:1298-gtest into d466667 on numenta:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 39804e1 on utensil:1298-gtest into d466667 on numenta:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 091e5b5 on utensil:1298-gtest into d466667 on numenta:master.

@rhyolight
Copy link
Member

Blocked by #1264.

@rhyolight rhyolight changed the title Adjust nupic for the merge of numenta/nupic.core#179 Run C++ nupic.core tests through new gtest interface Oct 3, 2014
@utensil
Copy link
Member Author

utensil commented Oct 4, 2014

@rhyolight commented 10 hours ago

Blocked by #1264.

As the C++11 commits in nupic.core are reverted, this is not necessarily blocked by #1264 anymore.

@oxtopus
Copy link
Contributor

oxtopus commented Oct 6, 2014

This one's 👍 once .nupic_modules is updated to the right fork.

@utensil
Copy link
Member Author

utensil commented Oct 7, 2014

I was confused by the git history at first and eventually figure this out.

There was an attempt by oxtopus to merge my branch at #1408 but somehow failed the Mac build. And then oxtopus used a better method to link gtest: instead of linking gest built in nupic.core, build gtest in nupic itself again, which would avoid implicit dependency on nupic.core internal build structure. This work is at #1409 and already merged into master while I was sleeping 😴

But there was one piese missing in #1409, the cpp_unit_tests is still referring to testeverything instead of the new name unit_tests.

So after I resolved conflicts with numenta:mater, this PR has become a one line PR...

@utensil
Copy link
Member Author

utensil commented Oct 7, 2014

Closing this and go to #1413

@utensil utensil closed this Oct 7, 2014
@utensil utensil deleted the 1298-gtest branch October 7, 2014 03:49
@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.

Use new gtest interface in nupic.core
5 participants