Runloop: extracts parts and atomize state access

[MOZGIII]

This PR refactors the main runloop body into smaller parts, and atomizes access to the variables by effectively hand-rolling the explicit closures as params.

This change helps by moving the runloop implementation from a waterfall of code to a more structured sequence of fn calls with explicit parameter captures, thus enabling high-level reasoning and overview.

The code (for now) is organized into two major pieces: parts and ops.

• parts are what's going to hold the chunks of runloop, as in they were, pretty much following the linear logic and structure of the extracted code;
• ops are the bits that were corresponding to the reusable operations previously residing at impl Runloop { ... }, and accessible via self

The access to both runloop local variables and self variables is now explicitly provided via params. This can a bit verbose, but this verbosity also provides an intuitive understanding as to how complex a block of code really is, and an explicit overview of what data it can access. We will rely on this later when we will be analyzing usage patterns and encapsulation designs for data used by the runloop and shards.

Further refactors are to be be added on top of this.
Among other things, notable things to do next are:

• eliminate the RunLoop type altogether, and move to running the loop via a standalone fn;
• before going into the actual loop, the runloop does a lot of extra setup work, spawns other loops and thread, and generally does things that are more fitting for the setup code than for the actual runloop; these parts should be separated into their own composable setup routines, and sometimes into whole separate subsystems / loops; this work should be accompanied by additional unit-testing of the newly separated and testable components;
• switch from raw types (like Uuid) to newtypes (like LockId, InstanceId, ExecutionId, NodeId, etc) for added type-safety; together with this, give the variables more contextually-specific names - like lock_uuid should be this_execution_lock_id;
• use nonempty-collections to straighten out the internal logic.

---

Why switch from passing vars via self to via Params?

1. Before, it was unclear what state each fn call involved when reading the code; now it is immediately clear.
2. This setup is a prerequisite for expressing the execution flow in terms of continuations, where each part depends on the results from the previous part. I didn't yet do the semantic analysis to confirm this is something we'd want here, but it makes sense that it would be the case intuitively.

---

To do:

• extracts parts and ops
• cleanup dead code
• review the results, optimize the internal APIs, naming patterns and code placement
• fix issues discovered by python tests
• add some unit-tests
• document the purpose of each part and what does it do
• collapse the contexts to be a single parameter for non-owned variables (params)
• self code-review

---

Note this PR adds a respectable 1.5% test coverage; in actuality there are new tests that are catching certain things that have already been covered by python tests, however that was more accidental than intentional; we explicitly add tests (at the unit level) to ensure the small logic bits we now have work as intednded.

[github-actions]

Coverage Report

Python Coverage Metric Coverage Lines 75.8% Branches 57.8% Download HTML Report

Rust Coverage Metric Coverage Lines 61.6% 🟢 (+1.6%) Branches N/A Download HTML Report

_{Compared to main branch}

[piercefreeman]

This refactor is shaping up really nicely. Leaving my initial comments below.

[piercefreeman]

Seems like we're sharing the same basic constructors across all these test functions - a helper function that returns the default params (or a test-scoped extension to Params that allows for construction without these values) might make this code cleaner.

[MOZGIII]

6283c11 (this PR) how is this? If this looks good enough I'll distribute the same approach to the rest of the tests

[piercefreeman]

I agree the harness approach is cleaner (at least for these cases where we have to initialize a pretty large payload of params that are redundant across test functions).

I'm unclear on the separation of concerns between just directly mutating the harness properties via a harness.executor_shards = xyz (which you do to override the dicts) and the arguments that are intended to go in the params() signature. Would it make sense to just have "default" values for all of the parameters and go with harness assignment for them all? Or are we only expecting to house empty-collections within the harness and everything else is intended to be a params() param?

[MOZGIII]

The separation of concerns is arbitrary; I think it is a mistake. Let me iterate a bit more on this to we can simplify; we can reduce the harness to just a bag of values really.

We probably don't want to generalize and reuse it among the tests for different parts - as it would needlessly broaden the scope of the tests. So each part would get a test harness that only accepts overrides via fields, and a direct params conversion.

[MOZGIII]

On the separation of concerns - there is a bit of a practical thing though - the params instance holds some non-Copy fields by value by design, in this case step; so, those would still have to be passed explicitly via params argument. Also the mocks just have shorter access paths if they're provided from the outside - but there's actually no practical need to ever construct them separately - so I'm inlining them as well for now; if there's a need we can move them back to .params args.