Cleanup some e2e test configurations

[ahrtr]

See #14360 (comment)

Notes:

1. compactPhysical in ctlCtx and withQuota aren't used at all, they are dead code.
2. quotaBackendBytes in ctlCtx isn't used either. Instead, users (test cases) set the QuotaBackendBytes directly.
3. If we want to enable a feature (e.g. initialCorruptCheck) by default, then a best practice is to define the field as "DisableXX", such as DisableInitialCorruptCheck bool. Its value will be false by default if not set.

Signed-off-by: Benjamin Wang wachao@vmware.com

[codecov-commenter]

Codecov Report

Merging #14389 (17b83ea) into main (e408285) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #14389      +/-   ##
==========================================
+ Coverage   70.73%   70.78%   +0.05%     
==========================================
  Files         455      455              
  Lines       37161    37161              
==========================================
+ Hits        26285    26306      +21     
+ Misses       9362     9338      -24     
- Partials     1514     1517       +3     

Flag	Coverage Δ
all	70.78% <ø> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
client/v3/experimental/recipes/queue.go	58.62% <0.00%> (-6.90%)	⬇️
client/v3/namespace/watch.go	87.87% <0.00%> (-6.07%)	⬇️
pkg/adt/interval_tree.go	84.71% <0.00%> (-2.51%)	⬇️
server/lease/leasehttp/http.go	64.23% <0.00%> (-1.46%)	⬇️
server/etcdserver/api/v3rpc/maintenance.go	72.60% <0.00%> (-1.37%)	⬇️
server/embed/serve.go	53.21% <0.00%> (-0.86%)	⬇️
client/v3/op.go	75.09% <0.00%> (-0.77%)	⬇️
pkg/proxy/server.go	60.67% <0.00%> (-0.34%)	⬇️
server/etcdserver/api/v3rpc/watch.go	85.61% <0.00%> (-0.34%)	⬇️
server/etcdserver/server.go	82.67% <0.00%> (-0.23%)	⬇️
... and 15 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ahrtr]

cc @serathius @spzala @ptabor @clarkfw

[serathius]

I don't think this direction is much better. This is bad way to handle defaults, negating the configuration fields again makes it harder to track and understand.

Instead I would prefer that we separate struct definition from defaults. This way we can make the configuration struct more similar to "production" one and just have tests having different defaults.

Example of what I mean:

type  EtcdProcessClusterConfig struct {
...
  InitialCorruptionCheck bool
...
} 

func DefaultEtcdProcessClusterConfig() *EtcdProcessClusterConfig {
  return &EtcdProcessClusterConfig{
    InitialCorruptionCheck: true,
  }
}

func TestCorruption(t *testing.T) {
  config := DefaultEtcdProcessClusterConfig()
  config.InitialCorruptionCheck = false
  ...
}

Later we might decide to shorten shorten code by adding options for DefaultEtcdProcessClusterConfig, but this is separate issue.

[ahrtr]

I don't think this direction is much better. This is bad way to handle defaults, negating the configuration fields again makes it harder to track and understand.

Instead I would prefer that we separate struct definition from defaults. This way we can make the configuration struct more similar to "production" one and just have tests having different defaults.

Thanks. Actually it's a tradeoff. The good side is that we can leverage golang build-in feature of zero value (e.g. false for bool), so that the implementation can be simplified because we need to do nothing if we just want to use the default value; while the bad side it's counterintuitive (not disable == enable), and it isn't consistent with the product code just as you pointed out.

Let me follow your suggestion in this PR. I will rollback the change to InitialCorruptCheck, and we can address it in a separate PR.

[ahrtr]

Done.