Test is passing for wrong reason

[utkarshg6]

The test route_query_responds_with_none_when_asked_for_one_hop_round_trip_route_without_consuming_wallet_when_back_route_needs_two_hops is passing due to an unintended configuration rather than the intended behaviour.

It claims to be stimulating a failed query because of a hop-count problem when the query is actually failing because there are no full neighborships. If the test uses .add_arbitrary_full_neighbor instead of .add_arbitrary_half_neighbor, and the RouteQueryResponse turns out to be None, then the test is doing what the name suggests.

If you try replacing the function mentioned above, you'd find that we receive a RouteQueryResponse but if the test was doing as the name suggests, we shouldn't be receiving any response.

[dnwiebe]

Perhaps before spending too much effort on making this test pass correctly, figure out exactly what it's supposed to be testing and whether it has become obsolete (or perhaps was wrong-headed in the first place).

It's making a triangle of Nodes and then asserting that no over-and-back route with a minimum hop count of 1 can be found through them. That seems strange to me: I could find at least four routes:

A -> B, B -> A
A -> B, B -> C -> A
A -> C -> B, B -> A
A -> C -> B, B -> C -> A

Maybe the test should just be deleted.

[dnwiebe]

There may be other problems with this test: for one thing, it claims to use no consuming wallet when it looks as though the subject definitely does have a consuming wallet. Also, it talks about needing a two-hop return route, but I can't see why that would be necessary. Basically, review this test and correct what needs to be corrected.

[czarte]

We have found this test as obsolete and we decided to delete it.

Dan's note:
While chasing down some puzzling test behavior, we discovered something new about our reduce-min-hops campaign. Before we did the min-hops change to allow min-hops to shrink below 3, the code was assuming that all one-hop routes were Gossip routes, and therefore should have no consuming wallet associated with them; all multi-hop routes were data routes and required a consuming wallet.

Now, though, we have one-hop routes that are data routes and need to have consuming wallets so that the exit Node can be paid, but they don't require consuming wallets because the routing code thinks they're Gossip routes.

This isn't an immediate problem, because our code is dropping consuming wallets into all the data routes anyway (I think); but it's something we didn't notice when we shrank the min-hops requirement, and we should keep it in mind.