-
Notifications
You must be signed in to change notification settings - Fork 291
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
test: Add more unit tests for add_to_list
.
#2531
Conversation
6f9aeed
to
c4f92bf
Compare
a754eff
to
f229de2
Compare
f229de2
to
8fc557e
Compare
8fc557e
to
ea7570b
Compare
4f43c3b
to
a910f04
Compare
02ecdb9
to
9b2241e
Compare
c378cf2
to
b4d7bf4
Compare
5e56eba
to
c68aeac
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2531 +/- ##
==========================================
+ Coverage 68.97% 69.12% +0.14%
==========================================
Files 89 96 +7
Lines 27784 27983 +199
==========================================
+ Hits 19164 19343 +179
- Misses 8620 8640 +20 ☔ View full report in Codecov by Sentry. |
22db489
to
8d3f656
Compare
Nit: modulo doesn't result in a uniform distribution, it gets skewed. Might not matter for testing though. |
8d3f656
to
1d4f10b
Compare
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.
Reviewable status: 0 of 1 approvals obtained (waiting on @Green-Sky and @nurupo)
toxcore/crypto_core_test_util.cc
line 51 at r2 (raw file):
Previously, nurupo wrote…
Nit: modulo doesn't result in a uniform distribution, it gets skewed. Might not matter for testing though.
C++' random library has random number generators andstd::uniform_int_distribution()
too, wonder why they weren't used here.
Added a comment. I'll test/benchmark the C++ stdlib stuff later. For now, this is a very simple and fast implementation intended to be exactly that.
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.
Reviewable status: 0 of 1 approvals obtained (waiting on @Green-Sky and @nurupo)
toxcore/crypto_core_test_util.cc
line 51 at r2 (raw file):
Previously, iphydf wrote…
Added a comment. I'll test/benchmark the C++ stdlib stuff later. For now, this is a very simple and fast implementation intended to be exactly that.
I guess LCG is the same as I put above. Maybe I'll refactor it tomorrow, or otherwise it'll be done in the next round of refactorings.
ea3a53c
to
00c81fb
Compare
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.
Reviewable status: 0 of 1 approvals obtained (waiting on @Green-Sky and @nurupo)
toxcore/crypto_core_test_util.cc
line 51 at r2 (raw file):
Previously, iphydf wrote…
I guess LCG is the same as I put above. Maybe I'll refactor it tomorrow, or otherwise it'll be done in the next round of refactorings.
I've decided to use C++ :P. Done.
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.
Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: 0 of 1 approvals obtained (waiting on @Green-Sky)
FYI was just pointing out something I noticed while skimming through the PR, don't plan on seriously reviewing it so don't hold the PR waiting on my review approval. |
00c81fb
to
afc38f2
Compare
This change is