Expose the STUNReachability experiment #137
Conversation
nettests/psiphon.go
Outdated
| if tk["failure"] != nil { | ||
| testKeys.IsAnomaly = true | ||
| testKeys.Failure, ok = tk["failure"].(string) | ||
| failure, ok := tk["failure"].(*string) |
There was a problem hiding this comment.
I found out that this pattern needed to be changed when testing stunreachability
There was a problem hiding this comment.
Is the same change perhaps needed also for BootstrapTime?
There was a problem hiding this comment.
No: BootstrapTime is not nullable. The issue here is that casting to *string and casting to string is not the same and we need to use the precise type. As for BootstrapTime it's float64 so we're good.
Make sure dependencies of probe-engine are at the same exact version to which is probe-engine, to avoid any possible issue.
bassosimone
changed the title
|
@hellais I think it's fine that CLI users see it. What do desktop user see? |
|
I tested this in the desktop app and it appears that it will in fact ignore the stun test result, but the counter for the number of tools is inconsistent (it says 3 instead of 2):
I think the best way to go about doing this is to omit writing any sort of information about the results of this test to database at all and ensure that the output is identical to the output without this test. |
|
We'll put this PR on hold until we fix the issue of bad names in the database. |
|
@hellais I've changed the PR so that we don't run STUNReachability. So, we can get the benefits of the latest probe-engine, but we are not running the test. (Deleting the existing code or opening a new PR looked like not needed to me; I'm open to create a new PR if you consider instead that doing that would be more proper form for proceeding.)
I think this should not prevent merging this PR. We should instead start integrating it and then possibly perform another small update to make sure we've a good DB. |
bassosimone
changed the title
|
I created a new epic and child issues for this: |
bassosimone
changed the title
|
This branch now just contains the STUNReachability code. The part related to updating the version of ooni/probe-engine and to fixing Psiphon code landed as part of #138 |
|
I have added STUN to the experimental nettest group in #243. This PR is now obsolete. |
Closes ooni/probe#1185.