Deprecate manually adding simulated sled agents #7375
Conversation
Change tests that manually add simulated sled agents to instead use the new `extra_sled_agents` macro parameter. This was also pulled out of work as part of testing for oxidecomputer/crucible#1594.
nexus/src/app/sagas/test_helpers.rs
Outdated
| pub(crate) fn select_first_alternate_sled( | ||
| db_vmm: &crate::app::db::model::Vmm, | ||
| other_sleds: &[(SledUuid, omicron_sled_agent::sim::Server)], | ||
| other_sleds: &[&omicron_sled_agent::sim::Server], |
There was a problem hiding this comment.
Now that there's an all_sled_agents function on the test context, I think this whole function might boil down to
cptestctx.all_sled_agents().find(|sa| sa.id != db_vmm.sled_id).expect("need at least one other sled")which could either be used as a function body here or substituted into the places where it's called (there are only three in my local clone).
There was a problem hiding this comment.
Done in b7de7c6, thanks :)
| async fn test_migration_source_completed_succeeds( | ||
| cptestctx: &ControlPlaneTestContext, | ||
| ) { | ||
| let _project_id = setup_test_project(&cptestctx.external_client).await; | ||
| let other_sleds = test_helpers::add_sleds(cptestctx, 1).await; | ||
| let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect(); |
There was a problem hiding this comment.
Nit, take it or leave it: we write this a lot...maybe ControlPlaneTestContext should just have a method that returns all of the extra sled agents?
There was a problem hiding this comment.
Also, perhaps we could have the ControlPlaneTextContext return that as a slice, using &self.sled_agents[1..], so that callers don't need to always iterate over the sled agents and collect them?
Edit: Oh, disregard that last bit, I see that all_ssled_agents is returning the sa.server(), so it does have to be an iterator. Carry on.
There was a problem hiding this comment.
Done in b7de7c6, thanks :)
Change tests that manually add simulated sled agents to instead use the new
extra_sled_agentsmacro parameter.This was also pulled out of work as part of testing for oxidecomputer/crucible#1594.