addressing more comments related to metrics pr #3126
Conversation
| @@ -75,7 +76,8 @@ public void beforeAll(final ExtensionContext context) throws Exception { | |||
| maintainConfigs = List.of( | |||
| new CorrectExplains(new EmbeddedConfig()), | |||
| new CorrectMetrics(new EmbeddedConfig()), | |||
| new CorrectExplainsAndMetrics(new EmbeddedConfig()) | |||
| new CorrectExplainsAndMetrics(new EmbeddedConfig()), | |||
| new ShowPlanOnDiff(new EmbeddedConfig()) | |||
There was a problem hiding this comment.
The show-plan-on-diff config seems different enough that you wouldn't want to run it in the same step as running explain or metrics correction
There was a problem hiding this comment.
The configs in here do not run in succession. You can use the maintain annotation to pick the configs from here you want to run.
There was a problem hiding this comment.
Oh okay, I see. I was confused by the flow here, but I think I understand:
- The
maintainConfigslists all possible configurations that you'd want to run to maintain the files. In this way,ShowPlanOnDiffis a bit different, because it never updates any of the pre-existing files, but it is kind of aDebugsetting--one that you wouldn't want to run normally. - The annotation has a
valuethat it sets - When choosing the set of tests to run, it filters out the
maintainConfigsthat don't align with its value
There is still something about this that feels a bit wrong, design wise. (I agree that it achieves the intended result.) Like, it feels like the annotation choice should result in the appropriate config being created, rather than filtering out all of the configs that aren't the one we want. It's also a bit strange that it happens when the configuration is invoked rather than when the configuration is created. But I see what you're getting at here.
No description provided.