-
Notifications
You must be signed in to change notification settings - Fork 65
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
Update initial cpp unittests #128
Conversation
@gpucibot merge |
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.
Thanks Gigon! 😄
Had a couple suggestions below
benchmarks/config.h
Outdated
@@ -42,6 +43,38 @@ struct AppConfig | |||
std::string benchmark_color; // {auto|true|false} | |||
std::string benchmark_counters_tabular; | |||
std::string v; // <verbosity> | |||
|
|||
std::string get_input_path(const char* default_value = "generated/tiff_stripe_4096x4096_256.tif") const |
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 default_value
be std::string
? Similar lines below
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.
Thanks! Updated to const std::string
std::string input_file = "test_data/private/generic_tiff_000.tif"; | ||
std::string test_folder; | ||
std::string test_file; |
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.
We may want to revisit at some point and look at using std::filesystem::path
. Though that requires C++17 and we were already using std::string
. So just a future note
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.
Thanks!
Created a issue for following up:
b629a0a
to
229abaf
Compare
229abaf
to
8a7dd7d
Compare
@gpucibot merge |
Update existing unit test code to use generated test image and allow external image specified by the parameter.
Updating
run
script to pre-generate the necessary test images and running C++ unit tests would be set up in a separate PR.