-
Notifications
You must be signed in to change notification settings - Fork 3
Best test framework #19
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
base: master
Are you sure you want to change the base?
Conversation
|
The reason you can't find a good answer for the resolution of clocks is that it varies, there isn't a way to get 'exactly' ms precision, you usually get something in the ballpark. But these tests shouldn't be too crazy, so ball park is good enough. |
FredTheDino
left a comment
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.
It's a nice start, and it will work wonders for stability
| namespace Test { | ||
|
|
||
| bool test_slow_test_false() { | ||
| int a = 0; |
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.
Maybe a sleep? Busy wait seems like a bad idea. It might also be optimized out by the compiler and might infact be quite a fast test. But of course that depends on the compiler.
|
Quoting Edvard Thörnros (2020-02-08 22:17:29)
In src/test/test_test.cpp:
> @@ -0,0 +1,17 @@
+namespace Test {
+
+bool test_slow_test_false() {
+ int a = 0;
Maybe a sleep? Busy wait seems like a bad idea. It might also be optimized
out by the compiler and might infact be quite a fast test. But of course
that depends on the compiler.
I used them to see that the formatting worked as it should, so in a
sense they're tests-tests. They probably shouldn't be part of the
finished system.
In src/test/test_main.cpp:
> + double elapsed = end.tv_nsec >= start.tv_nsec
+ ? (end.tv_nsec - start.tv_nsec) / 1e6 + (end.tv_sec - start.tv_sec) * 1e3
+ : (start.tv_nsec - end.tv_nsec) / 1e6 + (end.tv_sec - start.tv_sec - 1) * 1e3;
Yes, that is correct. Good research! But it feels like there should be a
simpler way...
I agree, I'll try chrono since it looks like the recommended C++ way.
|
|
I think I prefer chrono over clock_gettime, it feels clearer what's happening. Let me know what you think. |
FredTheDino
left a comment
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.
LGTM, nice to have some more tests!
| Result math_eq() { | ||
| return SKIP; | ||
| } |
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.
Put in some numbers and see if it actually works. It checks equality with a margin, usefull for floating point compares.
src/test/test_main.cpp
Outdated
| printf(RED "unexpected successes:" RESET " %3d\n", (failed_expected - failed)); | ||
| printf(YELLOW "expected failures:" RESET " %3d\n", failed); | ||
| printf(YELLOW "skipped:" RESET " %3d\n", skipped); | ||
| printf("%d tests in %.2f ms\n", size - skipped, (elapsed.count() / 1e6)); |
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.
chrono, I personally wouldn't use it, but that doesn't mean its bad. This isn't critical code so I'm fine with this. It can get messy when you want to change to different time-formats, but you kinda skipp it here. But it is the "C++ way", and the usage looks good.
FredTheDino
left a comment
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.
I should place a reminder for pressing the submit button...
`return SKIP` already does what the macro did, so now you only have to specify skip in one place instead of two.
Also, some formatting
Commit ef67422 shows how failing tests are handled and shown. I figured printing tests that pass is unneccesary since you only really care about the tests that fail. The time is in CPU-time since
clock_gettimeis portable enough (supports both linux and mac) and has better-than-millisecond resolution. (At least I think it does, it's not easy to find a straight answer to which resolution certain methods has that's true for all platforms.)