Skip to content

Conversation

@jgallagher
Copy link
Contributor

@jgallagher jgallagher commented Nov 12, 2025

I'd like to add a couple new fields to BlueprintSledConfig, and doing so broke a variety of tests in annoying ways. Many of those tests have required annoying changes in the past, too. This PR doesn't add any new fields, but instead attempts to clean up those tests so changes to Reconfigurator structures are less likely to cause problems (and hopefully any problems will be easier to fix). A couple techniques:

  • Some tests that were creating raw blueprints by hand now use either BlueprintBuilder or ExampleSystemBuilder
  • Replaces BlueprintBuilder::build_empty_with_sleds{,_seeded} with BlueprintBuilder::build_empty{,_seeded}. The former added some sleds with nothing on them; the latter adds no sleds at all. I updated all the callers of build_empty_with_sleds to either use build_empty (actually fine for most of them) or the example system infrastructure.

This does add a few new helper methods on real types; e.g., BlueprintBuilder::sled_add_zone_boundary_ntp() (for directly adding a boundary NTP instead of promoting an existing internal NTP zone). But currently only tests call these new methods, and I don't think they're prone to misuse; e.g., sled_add_zone_boundary_ntp() checks and fails if called on a sled that already has an in-service NTP zone of either flavor.

(This is staged on top of #9392 to pick up that fix for main.)

// If used for RSS tests (most of the time!), the blueprint built by this
// builder will need to have its `parent_blueprint_id` manually set to
// `None` to erase the link to the empty parent.
fn blueprint_builder_with_empty_parent(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still kinda janky, and was earlier in the stack of work that produced this PR. I think I could go through these tests again and replace most or all uses of this with more ExampleSystemBuilder usage, if folks think that'd be better.

Base automatically changed from sunshowers/spr/reconfigurator-cli-update-snapshots to main November 13, 2025 00:29
@jgallagher jgallagher force-pushed the john/cleanup-blueprint-building-in-tests branch from e7417cd to 263cd1f Compare November 18, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants