Add test name to e2e cluster members #14292
Conversation
This should aid in debugging test flakes, especially in tests where the process is restarted very often and thus changes its pid. Now it's a lot easier to grep for different members, also when different tests fail at the same time. The test TestDowngradeUpgradeClusterOf3 as mentioned in etcd-io#13167 is a good example for that. Signed-off-by: Thomas Jungblut <[email protected]>
| "go.uber.org/zap" | ||
| "go.uber.org/zap/zaptest" | ||
|
|
||
| "go.etcd.io/etcd/server/v3/etcdserver" |
There was a problem hiding this comment.
@tjungblu thanks, changes lgtm but I am curious why are you changing import order here and in other files? The current order is as per overall convention used in the project (i.e. third party packages follow internal ones..) and we should keep it that way unless I am missing anything.
There was a problem hiding this comment.
@tjungblu can you please look at my small comment above?. Your PR looks good otherwise and we can merge it.
There was a problem hiding this comment.
I won't hold the PR due to the ordering changes in the import which seems unnecessary but can be reordered separately.
There was a problem hiding this comment.
Usually we shouldn't change the existing code (including this case, the order of the import entries and the blank line) without explicit reason.
But on the other hand, it seems that it isn't a big deal on the change in this case. Either keeping it as it's for now or reverting the change works for me.
I suggest to keep it as it's for now.
Then this PR is good to go, and defer to @spzala to make the final decision and merge the PR.
This should aid in debugging test flakes, especially in tests where the process is restarted very often and thus changes its pid.
Now it's a lot easier to grep for different members, also when different tests fail at the same time.
The test
TestDowngradeUpgradeClusterOf3as mentioned in #13167 is a good example for that.Signed-off-by: Thomas Jungblut [email protected]