test: unit tests improvements #3281
Conversation
lalo
commented
Sep 2, 2021
lalo
commented
- remove print from explore_test.cc
- shift things around from tutorial_test.cc to simulator.h
- add callback hook to simulator
1) remove print from explore_test.cc 2) shift things around from tutorial_test.cc to simulator.h 3) add callback hook to simulator
| const auto str_pdf = to_string(the_pdf); // 2-3.5:0.0666667,3.5-4.5:0.8,4.5-6.2:0.0588235 | ||
| // avoid float precision compare | ||
| BOOST_CHECK_EQUAL(str_pdf.find("2-3.5:"), 0); | ||
| BOOST_CHECK(str_pdf.find(",3.5-4.5:0.8,4.5-6.2:") > 0); |
There was a problem hiding this comment.
This wont be resilient to float differences. Is that okay? Also if the to_string changes then this test will break
There was a problem hiding this comment.
well its all generated from right above, any suggestions on how to replace that print? or just delete or what
There was a problem hiding this comment.
You could pick out the bits of it and assert that segment exists. It's way more work and not really worth it. I just wanted to point it out. Let's not worry about it
There was a problem hiding this comment.
if it passes it stays, if not we can later revisit. It is already wrong printing random stuff.
| { | ||
| // maps an int: # learned examples | ||
| // with a function to 'test' at that point in time in the simulator | ||
| using callback_map = typename std::map<int, std::function<bool(vw*)>>; |
There was a problem hiding this comment.
Typename is probably not needed here
There was a problem hiding this comment.
it will be later i think, or what makes you say that?
There was a problem hiding this comment.
std::map is not ambiguous so you don't need to tell the compiler is a type? I'm not 100% so leaving it here is fine
There was a problem hiding this comment.
yea the benefit is low, i was thinking for default arg to some function. It looks a bit cleaner with a short typedef
There was a problem hiding this comment.
typename shouldn't be needed since there isn't a need to specify that this is a type, I think it is mostly used with templates to distinguish any ambiguity
|
related: #3252 |
|
|
||
| std::vector<float> _test_helper(const std::string&, int = 3000, int = 10); | ||
| std::vector<float> _test_helper_save_load(const std::string&, int = 3000, int = 10); | ||
| std::vector<float> _test_helper_hook(const std::string&, callback_map&, int = 3000, int = 10); |
There was a problem hiding this comment.
where is this being called from?
There was a problem hiding this comment.
automl PR for more examples:
vowpal_wabbit/test/unit_test/automl_test.cc
Line 162 in 3528eeb
lalo
force-pushed
the
fix_ut
branch
2 times, most recently
from
332bddd to
1053b12
Compare
4 participants
