Skip to content

tor-circmgr: Remove AbstractSpec and FakeSpec.

wesleyac requested to merge wesleyac/arti:circmgr-remove-abstractspec into main

AbstractSpec and FakeSpec actually make testing more difficult, since they prevent using FakeBuilder in code that relies on the concrete TargetCircUsage and SupportedCircUsage types. Removing them means FakeBuilder can be used in more places, and also means that the test code is closer to the real code, since TargetCircUsage and SupportedCircUsage are now exercised directly in more tests.

This did require making one change to a test, which I think was previously testing behaviour that was true for FakeSpec but not for the real code:

The mgr::test::isolated test previously asserted that, in the case where three circuits were requested, two with isolation and one without, the non-isolated circuit would be shared with one of the isolated circuits.

This was allowed by the FakeSpec::supports function. However, in the actual code, the path is as follows:

  • AbstractCircMgr::get_or_launch
  • AbstractCircMgr::prepare_action
  • CircList::find_open
  • AbstractSpec::find_supported
  • abstract_spec_find_supported
  • OpenEntry::supports
  • SupportedCircUsage::supports
  • StreamIsolation::compatible_same_type

StreamIsolation::compatible_same_type checks owner_type, which is always zero for non-isolated streams and always non-zero for isolated streams, meaning that a isolated stream will never be compatible with a non-isolated stream. The seems like desirable behaviour, so I simply modified the test to make four connections, two isolated and two not, and checked that the isolated streams never share any circuits, and that the two non-isolated streams use the same circuit. As far as I can tell, this is the intended behaviour in the existing code.

Edited by wesleyac

Merge request reports