[tor-dev] Chutney code refactoring

[Nick Mathewson]

On Wed, Apr 15, 2020 at 11:45 PM c <c at chroniko.jp> wrote:

Skipping over some design stuff that seems reasonable.

 [...]
> Here is what I have been considering lately, with the code overall. I
> hope that laying it out in points will help both myself and others try
> to divy up what could be improved about the code, work on it piecewise,
> and make sure nothing is reimplemented incorrectly. I know I'm new to
> the code and thus have not looked into it thoroughly enough to
> understand everything about it, but again, something in the back of my
> head is telling me that if it's this difficult to understand now, then
> it can stand to be changed. So please take my criticism lightly and
> only where it applies. I could be wrong about some things as well. But,
> as a testing suite, this is the place we absolutely need to be sure we
> get right, because with a hard-to-reason testing suite, we open
> ourselves up to bugs within the tests themselves.

Agreed totally.

> - Documentation especially for methods that the tests are supposed to
>   call. In `Node` class, there is a comment "Users are expected to call
>   these" with `__init__` and three other functions declared. `__init__`
>   is fairly self-explanatory but the other three could stand to have
>   the same docstrings that we are giving other functions. I think this
>   should be my first task, before trying to mess with anything that
>   moves. That way, we have a clearer overview of the testsuite's API,
>   and then we can work the implementation around that.

This seems plausible, though I would caution that we shouldn't expect
to find a great separation between the external and internal parts of
chutney right now.  It's been built up piece by piece over time
without a wholly consistent vision, so it probably doesn't have the
best isolation between its layers.

This documentation project would also IMO benefit from a high-level or
module-level overviews on what things exactly chutney does.  Some of
its tasks are well-documented, but others are less so.

If you want to work on this it would be helpful to maybe start by
listing (here or elsewhere) some places where you *don't* feel like
you could (or would want to) write documentation: those would be a
good target for devs who _have_ worked on Chutney before.

> - Once it's clear to all of us (meaning, not only the core Chutney
>   devs, but also outsiders such as myself) what the current interface
>   does, we can see the strengths of that interface, maybe some design
>   flaws if we deem any to exist, and some improvements we can make with
>   the interface. What I mean is: here is the place to make any
>   necessary adjustments to the API before we dive into implementation.

+1.  Though I think this is likely to be an ongoing change that we see
over time, since

> - Now we visit the implementation. By this point I (and other non-core
>   developers) should have a better understanding on what the tests
>   *should* be doing, that we can then proceed to verify the correctness
>   of the tests' implementations themselves. Here's where I would like
>   to revisit cleaning up the code so that we make use of functions to
>   abstract and "atomize" certain aspects of the code (like the
>   currently-unused function isBoostrapped), where we should introduce
>   more self-documenting names (as suggested, replace getAttribute names
>   with isAttribute names for when we want to return bool values), et
>   cetera.

Yes.  I think there are other steps that could help at this stage too,
including:
  * More tests for the various parts of chutney, and refactoring to
make them more testable.
  * Making chutney comply with one or more of the python style
checking tools that are out there.

> I'm outlining what *I* think the approach should be, and in hopes that
> everything else will line up and make more sense for us all. I do want
> to make sure I know what the code should be doing, because while I
> could inspect the code line by line and go through the logic branches
> myself, I'm going to be perfectly honest and say it gives me a
> headache :) I think it's much easier to reason about with
> compartmentalised, self-documenting (and docstringed) components, than
> to try to step through the code as it runs and take it in as one giant
> piece.
>
> Because, we're testing a complex piece of software (Tor). Tor itself
> can do a lot of things that can throw us off especially if we're less
> familiar with its implementation, yet. And that throws off our
> understanding of how Chutney is supposed to work, as well. A lot of the
> tests currently seem to be very conditional -- they pass or fail with
> little rhyme or reason, which is of course the best type of issues to
> debug :P -- so I don't see the test results helping me as well as
> refactoring the testsuite first.

This is an good vision for how Chutney should go; I really support this idea.

> Again, let me know if I have anything wrong, made any unjust
> assumptions, showed my ignorance, et cetera. I'm going into this boldly
> and I hope that's the right call.
>
> Caitlin
> _______________________________________________
> tor-dev mailing list
> tor-dev at lists.torproject.org
> https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev