rootnomad commented on PR #14176: URL: https://github.com/apache/cloudstack/pull/14176#issuecomment-6473425905
> I just wonder if this should be an extra test instead of a replacement. running in an admin shared network makes sense as test as well, does it? Thanks @DaanHoogland. I'd lean replacement here. In these two files the network isn't the thing under test, it's just scaffolding to get a VM running so the planner and the metrics APIs can be exercised. Deploying into the caller's default network (the admin shared network on Trillian) couples the test to global state: the admin router being healthy, and no earlier test having left that network in Shutdown, which is exactly what makes them flap (tid-16935, tid-16908, tid-16968). The test-account L2 network removes that coupling and the virtual-router dependency, so these only fail when the planner or metrics code actually breaks. Keeping a shared-network variant of these two would just re-add that flaky coupling to tests that aren't about network placement. If we want explicit coverage that deploying into a shared/admin network works, I think that belongs in its own dedicated test that sets the network up and tears it down, so it isn't at the mercy of leftovers, rather than riding on the planner/metrics smoke tests. Happy to write that as a follow-up if you think it's worth having. Does that sound reasonable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
