-
Notifications
You must be signed in to change notification settings - Fork 287
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
-DSOCKS_HOST_TEST and -DSOCKS_PORT_TEST for testing #2469
Comments
Settable from CMakeCache.txt would be nice so you don't have to recompile the library, or is that defacto already the case? |
there needs to be another way. There is a proxy specific test, that tests the proxy functionality. If you think it is missing coverage, that test should be expanded instead. |
Can you point me to the "proxy specific test, that tests the proxy functionality."? if it does what I'm asking here, I'll close this issue, but I can't see anything real in being in the ctests if it doesn't have a proxy server running during the tests. I think you're right: there needs to be another way. Turns out there is and iphy showed me how to spin up a VM with ipv6.disable=1 in Docker and then run tests using in Docker under qemu. But there is work to be done get apt to use the proxy. I tried to pull things altogeter on #2473 |
https://github.com/TokTok/c-toxcore/blob/master/auto_tests/proxy_test.c does not look exhaustive, but it is there. edit: this explains why it is not run on your machine: Lines 125 to 128 in 24b5472
|
Thanks - I've run it in the past, but came to the conclusion that a single call to tox_add_tcp_relay is a test of whether that call works and that's it. I don't think that is a test of whether the program connected to the network and the proxy works. In real life I find that tox_add_tcp_relay always returns true, even if the SOCKS proxy is not connected to the internet (wifi down). My toxygen code randomly picks 6 BS nodes and tox_add_tcp_relay them all, and then checks dht_connected, repeated every 5? seconds for up to 120? seconds. It's very rare to get the dht_connected in less than 30 seconds, more normal would be 45. Check what tox_add_tcp_relay and if it ever fails, but AFAIK this is a test of minimal use. The testing case over SOCKS gets harder for things like group_join; there on a fresh group, it's more like 45 minutes, and more likely to have a code failure than tox_add_tcp_relay as it changes often. |
While you're doing the socks flags, -DUSE_UDP_TEST should be added too, to make sure it's not evading tcp. |
Regarding https://github.com/TokTok/c-toxcore/blob/master/auto_tests/proxy_test.c I wrote in #2469 (comment)
Looking at the code to tox_add_tcp_relay in tox.c, as long as the relay host resolves, it doesn't even look for failure from add_tcp_relay:
which is what I see in practice. In the testsuite case it will never fail because the nodes it tests against are on localhost, which will always resolve. In other words, Tox's test of a tcp SOCKS proxy will always pass. PS: Dear Santa: A version of add_tcp_relay that loops through a set of tcp nodes and loops until dht_connected is true is very high on my wish list. |
@Green-Sky There's a blindspot in the project by relying on ctest. If the project supported Python bindings TokTok/py-toxcore-c#78 (comment) then it would be easier to use a proxy-aware testrunner written in Python: https://git.plastiras.org/emdee/toxygen_wrapper/src/branch/main/src/tox_wrapper/tests/tests_wrapper.py |
"proxy behind a firewall" - can you explain what that even means? the proxy test requires the test-proxy executable as parameter, which is started as an extra thread and provides both http and socks5 for the localhost test. the get back the the "firewalled" point, no where here should any traffic leave the system. (they dont bootstrap off the internet, but agains each other). also worth mentioning: the cmake/ctest proxy test does not work, as it does not supply the needed proxy executable as cli argument. c-toxcore/auto_tests/BUILD.bazel Lines 32 to 34 in e202341
|
I run a Tor proxy and I also run firewall rules that's what I call "proxy behind a firewall" . I could run a proxy without a firewall.
I could run the proxy test without being behind a firewall.
I missed that. But if you look in the Dockerfiles I think I saw it compiling a SOCKS proxy written in go and uses that. Of course it is an expected failure as I didn't provide it with the go compiled proxy :-,( so I'll go back to disabling that test, or provide it that proxy. So back to the original feature request: my theory is that if we add -DSOCKS_HOST_TEST and -DSOCKS_PORT_TEST for testing |
I don't get it. A firewall is setup at a network boundary. But the proxy test has no network boundaries, so a firewall would not come close the any traffic happening at the testcase... the proxy test sets up tox1, which has udp and tcp enabled (not bootstrapped) everything is happening localhost, how would a firewall against a proxy work here? and what would it do? |
For the proxy ctest you're right, I'll dig into the failure I'm getting and get the go proxy compiled. For the other tests; there are firewall rules in place such that nothing is coming in and only SOCKS to 9050 is going out (DNS to 9053 only). No problem up to test 50. then:
These will fail until you wire me up the changes for -DSOCKS_HOST_TEST and -DSOCKS_PORT_TEST for testng. Then the tests would run over the Tor proxy. Then we could get the results of all of the ctests "behind a firewall" over Tor. |
Please note that bootstrap and tcp_relay tests are the only tests that use the network. All the other ctests ate not testing real network-connected behaviour. I personally think that all the testing should be expanded as well so that real across-the-network testing is done in real world conditions. But that's better done in Python. If TokTok supported Python it could do a proxy-aware testrunner. But lets get this done at least. |
The ctest suite has never been run under a proxy. Could you add SOCKS_HOST_TEST and SOCKS_PORT_TEST for testing in auto_tests/auto_test_support.c - should be straightforward.
(And rename USE_IPV6 to USE_IPV6_TESTS while you're in there.)
The text was updated successfully, but these errors were encountered: