-
Notifications
You must be signed in to change notification settings - Fork 9
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
use EXPECT_RANGE_EQ in cli test #16
Conversation
I have updated also the Readme file. An owner of "seqan" needs to enable Travis access for this repository, before we can see CI results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two questions, otherwise it LGTM.
test/CMakeLists.txt
Outdated
@@ -39,6 +39,7 @@ macro (add_app_test test_filename test_alternative) | |||
# Add the test to its general target (cli or api). | |||
if (${test_alternative} STREQUAL "CLI_TEST") | |||
add_dependencies (${target} "${PROJECT_NAME}") # cli test needs the application executable | |||
target_include_directories(${target} PUBLIC "${SEQAN3_INCLUDE_DIR}/../test/include") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question: Why don't we have the source directory as a variable? Would make the path a bit more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this suggestion; I found a solution with SEQAN3_CLONE_DIR which makes it indeed better readable.
test/cli/cli_test.hpp
Outdated
@@ -6,6 +6,8 @@ | |||
#include <sstream> // ostringstream | |||
#include <string> // strings | |||
|
|||
#include <seqan3/test/expect_range_eq.hpp> // EXPECT_RANGE_EQ macro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention why this is a useful macro, so that other people are more likely to use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point!
This PR updates the SeqAn3 submodule and applies the EXPECT_RANGE_EQ macro in a command line interface test.
This PR closes seqan/product_backlog#4.