I think checking the validation rules as part of testing is important, but making that a per-test concern for handling it may be frustrating.
What do you think about extending in-JVM with a method that provides the previous config and version and expects the Instance implementation to upgrade it, and then leave the validation on? That would provide some documentation of how deprecated/removed options are expected to be migrated. It might also be worthwhile extending the upgrade tests so that we can also specify new configuration in addition to migrated configurated as the instance is upgraded. On Mon, Apr 4, 2022 at 8:43 AM Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: > Hi everyone, > In In-jvm tests there is a flag Constants.KEY_DTEST_API_CONFIG_CHECK to > opt-in/out from config checks done in YamlConfigurationLoader#check(). > The upgrade tests are currently set to ignore that check in order to work > around dealing with new properties in newer versions. What does this mean? > My understanding is that the lowest version from which we start an upgrade > test will load Config and use it for all versions. This means that our > checks will fail because in newer versions we set new config in > InstanceConfig that doesn't exist in the old version Config. If we opt in, > the tests will start failing because we need to remove parameters. > > I suggest we opt in by default to the checks so people consciously add > their config. What do I mean? Currently with the new types and names in > trunk, we exercise the backward compatibility and we set the old config > names and values that work with the previous versions and exercise the > backward compatibility but If I add a new name to set for config, this will > just be ignored silently and default Config is used. Test might even > pass...This is risky. > This was documented but I think the right course of action is to opt in > for checks and people ignore the checks in upgrade tests when they are sure > they add the right config and no typos, etc and they understand the > implications. The situation since that check was added has changed and if > we keep on adding more tests, I think this is important so we are sure we > test properly. > > Please let me know if I am wrong in my understanding and what you think. > > Best regards, > Ekaterina >
