nixos/test: colour machine names

[JJJollyjim]

Motivation for this change

This makes NixOS Test output involving many machines a lot easier to visually parse:

The machine-readable XML output is unaffected.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[tfc]

i find it a bit unsound that a caching mechanism is used on a function that uses side effects.

How about making the Machine constructor pick a color from the iterator and then simply store that for its runtime?

[tfc]

long term there is the rough plan to use python's logging frame work. a variant where a logger is injected into each machine's constructor would make a lot of sense, as this can be easily combined with coloring of machines.

[JJJollyjim]

@tfc it isn't unsound, lru_cache is documented to grow infinitely if no maximum size is specified. Making it in the machine was my original plan, but it gets messy, as the Machine doesn't control the Logger which

[JJJollyjim]

*which has maybe_prefix, which needs the colour

[Mic92]

I don't have a strong opinion on that that. The method suggested by @tfc sounds a bit easier to understand is is potentially less code. The color formatting could be even inlined in maybe_prefix.

[JJJollyjim]

@Mic92 the serial output doesn't come through maybe_prefix though, it happens directly in Machine.process_serial_output

[flokli]

Can we switch to the python logging framework (and by this, making tests more reliable) first, before adding new features?

Discussion started here: #86486 (comment)

[flokli]

I really like this, but this can be solved much more elegantly in the formatter.

[Mic92]

Is the python logging module even needed? print should be thread-safe, no? What is the issue with the current eprint method? I am actually a bit reluctant to ask a contributor to implement something completely different from what they change in a PR.

[JJJollyjim]

Yeah sorry I really don't have the context to implement the switch to logging, or arm hardware to debug flakeyness with it... if someone else moves to logging later on they can feel free to pull out the colouring if it's making it hard and I'll reimplement it

[flokli]

Is the python logging module even needed? print should be thread-safe, no? What is the issue with the current eprint method?

Things still weren't thread-safe, as I wrote in the linked comment.
I also experienced some random stalls, which are probably connected to the non-thread-safety.

I am actually a bit reluctant to ask a contributor to implement something completely different from what they change in a PR.

Agreed, but I'd also be hesitant adding more features into something we end up needing to refactor anyways. Maybe it's best to keep this PR open, until someone picked up the initial refactor.

[Mic92]

see #96254

[flokli]

@JJJollyjim with #96254 merged, would you be up to update this?

[JJJollyjim]

@ofborg test bitwarden.sqlite

[JJJollyjim]

oops, that turns out to want to rebuild a lot.

@ofborg test boot

[JJJollyjim]

ugh, i forget ofborg won't test attrsets

@ofborg test qboot

[andersk]

This had to be reverted (#96703) due to conflicts with the revert of #96254, which had broken several tests (#96699). I assume this should be resubmitted when that’s sorted out.