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

Catch test part 2 #2584

Merged
merged 7 commits into from Oct 6, 2020
Merged

Catch test part 2 #2584

merged 7 commits into from Oct 6, 2020

Conversation

jeffin143
Copy link
Member

Refer : #2523

#include <boost/test/unit_test.hpp>
#include "test_tools.hpp"
#include "catch.hpp"
#include "test_catch_tools.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this include needed here? I couldn't find usage of any function/macro from test_catch_tools.hpp

#include <boost/test/unit_test.hpp>
#include "test_tools.hpp"
#include "catch.hpp"
#include "test_catch_tools.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
src/mlpack/tests/cli_binding_test.cpp Show resolved Hide resolved
src/mlpack/tests/io_test.cpp Outdated Show resolved Hide resolved
Co-authored-by: Marcus Edel <marcus.edel@fu-berlin.de>
@rcurtin
Copy link
Member

rcurtin commented Sep 12, 2020

Hey @jeffin143, can you merge master in and fix the build? I think then this one is close to ready. 😄

@jeffin143
Copy link
Member Author

@zoq @rcurtin

Strange output on master branch

Screenshot 2020-09-30 at 12 39 33 AM

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Everything looks good to me here. Thanks @jeffin143! The VS15 failure (the C1060) is not something we can do much about, and the VS16 failure is fixed in another PR (I can't remember the number right now).

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit d1af492 into mlpack:master Oct 6, 2020
@rcurtin
Copy link
Member

rcurtin commented Oct 6, 2020

Another one in! 👍 I think we are getting close now. :)

This was referenced Oct 14, 2022
@rcurtin rcurtin mentioned this pull request Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants