Skip to content
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

Add random testing program. #917

Merged
merged 1 commit into from
Jun 23, 2018
Merged

Add random testing program. #917

merged 1 commit into from
Jun 23, 2018

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Jun 19, 2018

This can be used as a random stress test for toxcore.
Adjust the weights to make certain actions more or less likely.


This change is Reviewable

@iphydf iphydf added this to the v0.2.3 milestone Jun 19, 2018
@iphydf iphydf force-pushed the random-testing branch 15 times, most recently from d545881 to 1d18e24 Compare June 22, 2018 19:59
@sudden6
Copy link

sudden6 commented Jun 23, 2018

Reviewed 2 of 3 files at r1.
Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 80 at r1 (raw file):

struct Action {
  uint32_t weight;
  char const *title;

Maybe use std::string?


testing/random_testing.cc, line 188 at r1 (raw file):

Random::Random()
    : tox_selector(0, NUM_TOXES - 1),
      friend_selector(0, NUM_TOXES - 2),

I think you made this -2 to not select the executing function as target, right? We should allow that for additional coverage.


testing/random_testing.cc, line 211 at r1 (raw file):

  if (connection_status == TOX_CONNECTION_NONE) {
    std::printf("Tox #%u lost friend %u!\n", state->id(), friend_number);

Huh, didn't now this was a possible way to output something in C++, but why not use the std::cout stream?


testing/random_testing.cc, line 373 at r1 (raw file):

  uint32_t action_number;
  for (action_number = 0; action_number < MAX_ACTIONS; action_number++) {
    if (!all_connected(toxes)) {

Use && of nesting ifs?


Comments from Reviewable

@iphydf
Copy link
Member Author

iphydf commented Jun 23, 2018

Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 80 at r1 (raw file):

Previously, sudden6 wrote…

Maybe use std::string?

Maybe, but why?


testing/random_testing.cc, line 188 at r1 (raw file):

Previously, sudden6 wrote…

I think you made this -2 to not select the executing function as target, right? We should allow that for additional coverage.

You can't have yourself as a friend, so you can have at most NUM_TOXES - 1 friends.


testing/random_testing.cc, line 211 at r1 (raw file):

Previously, sudden6 wrote…

Huh, didn't now this was a possible way to output something in C++, but why not use the std::cout stream?

I like printf, and tox devs are familiar with it, so this code is more accessible to them. What's the reason for recommending iostream in this code, instead?


Comments from Reviewable

@sudden6
Copy link

sudden6 commented Jun 23, 2018

Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 80 at r1 (raw file):

Previously, iphydf wrote…

Maybe, but why?

No raw pointers and no accidentally placing \0 in the middle of a string


testing/random_testing.cc, line 188 at r1 (raw file):

Previously, iphydf wrote…

You can't have yourself as a friend, so you can have at most NUM_TOXES - 1 friends.

Right, sorry didn't think that through.


testing/random_testing.cc, line 211 at r1 (raw file):

Previously, iphydf wrote…

I like printf, and tox devs are familiar with it, so this code is more accessible to them. What's the reason for recommending iostream in this code, instead?

IMO it makes the code more readable to have the variables at the right place in the output string.

std::cout << "Tox #" << std::to_string(state->id()) << " lost friend " << std::to_string(friend_number) << "!" << std::endl;

But I think in this case familiarity of the other devs with printf is a better argument.


Comments from Reviewable

@iphydf
Copy link
Member Author

iphydf commented Jun 23, 2018

Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 80 at r1 (raw file):

Previously, sudden6 wrote…

No raw pointers and no accidentally placing \0 in the middle of a string

These are raw pointers to read only data from string literals. It doesn't really make sense to heap-allocate mutable strings for these. I'd make it constexpr if I could.


testing/random_testing.cc, line 211 at r1 (raw file):

Previously, sudden6 wrote…

IMO it makes the code more readable to have the variables at the right place in the output string.

std::cout << "Tox #" << std::to_string(state->id()) << " lost friend " << std::to_string(friend_number) << "!" << std::endl;

But I think in this case familiarity of the other devs with printf is a better argument.

Acknowledged.


testing/random_testing.cc, line 373 at r1 (raw file):

Previously, sudden6 wrote…

Use && of nesting ifs?

The two conditions aren't really related. The first one is "check if some toxes have disconnected", the second one is performing an action "reconnect", which results in termination if it fails. Thoughts?


Comments from Reviewable

@sudden6
Copy link

sudden6 commented Jun 23, 2018

testing/random_testing.cc, line 80 at r1 (raw file):

Previously, iphydf wrote…

These are raw pointers to read only data from string literals. It doesn't really make sense to heap-allocate mutable strings for these. I'd make it constexpr if I could.

One could make this string private and only allow a getter, but I guess this is overkill as well, so feel free to leave it.


Comments from Reviewable

@sudden6
Copy link

sudden6 commented Jun 23, 2018

Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 373 at r1 (raw file):

Previously, iphydf wrote…

The two conditions aren't really related. The first one is "check if some toxes have disconnected", the second one is performing an action "reconnect", which results in termination if it fails. Thoughts?

To me this structure looks like there's something missing inside the first if level so I prefer to use && to make it clear that nothing is missing. Also the && structure makes it easier to perform a de morgan transformation (!A and !B) == !(A or B) in your head.


Comments from Reviewable

@iphydf
Copy link
Member Author

iphydf commented Jun 23, 2018

Review status: 0 of 1 LGTMs obtained


testing/random_testing.cc, line 373 at r1 (raw file):

Previously, sudden6 wrote…

To me this structure looks like there's something missing inside the first if level so I prefer to use && to make it clear that nothing is missing. Also the && structure makes it easier to perform a de morgan transformation (!A and !B) == !(A or B) in your head.

Done.


Comments from Reviewable

@sudden6
Copy link

sudden6 commented Jun 23, 2018

:lgtm_strong:


Reviewed 1 of 1 files at r2.
Review status: :shipit: complete! 1 of 1 LGTMs obtained


Comments from Reviewable

This can be used as a random stress test for toxcore.
Adjust the weights to make certain actions more or less likely.
@iphydf iphydf merged commit cfff361 into TokTok:master Jun 23, 2018
@iphydf iphydf deleted the random-testing branch June 23, 2018 13:00
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Sep 5, 2018
v0.2.3
**Merged PRs:**

- [TokTok#951] Only run astyle if the astyle binary exists.
- [TokTok#950] Remove utils.c and utils.h from toxencryptsave build.
- [TokTok#949] Fixes to the imported sodium sources to compile without warnings.
- [TokTok#948] Add a MAX_HOSTNAME_LENGTH constant.
- [TokTok#947] Remove the format test.
- [TokTok#937] Add new Circle CI configuration.
- [TokTok#935] Add a test for double conference invite.
- [TokTok#933] Add Logger to various net_crypto functions, and add `const` to Logger where possible.
- [TokTok#931] Avoid conditional-uninitialised warning for tcp test.
- [TokTok#930] Disable UDP when proxy is enabled.
- [TokTok#928] Use clang-format for C++ code.
- [TokTok#927] Add assertions to bootstrap tests for correct connection type.
- [TokTok#926] Make NULL options behave the same as default options.
- [TokTok#925] Add tests for what happens when passing an invalid proxy host.
- [TokTok#924] Make the net_crypto connection state an enum.
- [TokTok#922] Clarify/Improve test_some test
- [TokTok#921] Beginnings of a TCP_test.c overhaul
- [TokTok#920] Add test for creating multiple conferences in one tox.
- [TokTok#918] Merge irungentoo/master into toktok
- [TokTok#917] Add random testing program.
- [TokTok#916] Fix linking with address sanitizer.
- [TokTok#915] Remove resource_leak_test.
- [TokTok#914] Make dht_test more stable.
- [TokTok#913] Minor cleanup: return early on error condition.
- [TokTok#906] Sort bazel build file according to buildifier standard.
- [TokTok#905] In DEBUG mode, make toxcore crash on signed integer overflow.
- [TokTok#902] Log only the filename, not the full path in LOGGER.
- [TokTok#899] Fix macOS macro because of GNU Mach
- [TokTok#898] Fix enumeration of Crypto_Connection instances
- [TokTok#897] Fix ipport_isset: port 0 is not a valid port.
- [TokTok#894] Fix logging related crash in bootstrap node
- [TokTok#893] Fix bootstrap crashes, still
- [TokTok#892] Add empty logger to DHT bootstrap daemons.
- [TokTok#887] Fix FreeBSD build on Travis
- [TokTok#884] Fix the often call of event tox_friend_connection_status
- [TokTok#883] Make toxcore compile on BSD
- [TokTok#878] fix DHT_bootstrap key loading
- [TokTok#877] Add minitox to under "Other resources" section in the README
- [TokTok#875] Make bootstrap daemon use toxcore's version
- [TokTok#867] Improve network error reporting on Windows
- [TokTok#841] Only check full rtp offset if RTP_LARGE_FRAME is set
- [TokTok#823] Finish @Diadlo's network Family abstraction.
- [TokTok#822] Move system header includes from network.h to network.c
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Sep 5, 2018
v0.2.3
**Merged PRs:**

- [TokTok#951] Only run astyle if the astyle binary exists.
- [TokTok#950] Remove utils.c and utils.h from toxencryptsave build.
- [TokTok#949] Fixes to the imported sodium sources to compile without warnings.
- [TokTok#948] Add a MAX_HOSTNAME_LENGTH constant.
- [TokTok#947] Remove the format test.
- [TokTok#937] Add new Circle CI configuration.
- [TokTok#935] Add a test for double conference invite.
- [TokTok#933] Add Logger to various net_crypto functions, and add `const` to Logger where possible.
- [TokTok#931] Avoid conditional-uninitialised warning for tcp test.
- [TokTok#930] Disable UDP when proxy is enabled.
- [TokTok#928] Use clang-format for C++ code.
- [TokTok#927] Add assertions to bootstrap tests for correct connection type.
- [TokTok#926] Make NULL options behave the same as default options.
- [TokTok#925] Add tests for what happens when passing an invalid proxy host.
- [TokTok#924] Make the net_crypto connection state an enum.
- [TokTok#922] Clarify/Improve test_some test
- [TokTok#921] Beginnings of a TCP_test.c overhaul
- [TokTok#920] Add test for creating multiple conferences in one tox.
- [TokTok#918] Merge irungentoo/master into toktok
- [TokTok#917] Add random testing program.
- [TokTok#916] Fix linking with address sanitizer.
- [TokTok#915] Remove resource_leak_test.
- [TokTok#914] Make dht_test more stable.
- [TokTok#913] Minor cleanup: return early on error condition.
- [TokTok#906] Sort bazel build file according to buildifier standard.
- [TokTok#905] In DEBUG mode, make toxcore crash on signed integer overflow.
- [TokTok#902] Log only the filename, not the full path in LOGGER.
- [TokTok#899] Fix macOS macro because of GNU Mach
- [TokTok#898] Fix enumeration of Crypto_Connection instances
- [TokTok#897] Fix ipport_isset: port 0 is not a valid port.
- [TokTok#894] Fix logging related crash in bootstrap node
- [TokTok#893] Fix bootstrap crashes, still
- [TokTok#892] Add empty logger to DHT bootstrap daemons.
- [TokTok#887] Fix FreeBSD build on Travis
- [TokTok#884] Fix the often call of event tox_friend_connection_status
- [TokTok#883] Make toxcore compile on BSD
- [TokTok#878] fix DHT_bootstrap key loading
- [TokTok#877] Add minitox to under "Other resources" section in the README
- [TokTok#875] Make bootstrap daemon use toxcore's version
- [TokTok#867] Improve network error reporting on Windows
- [TokTok#841] Only check full rtp offset if RTP_LARGE_FRAME is set
- [TokTok#823] Finish @Diadlo's network Family abstraction.
- [TokTok#822] Move system header includes from network.h to network.c
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants