I agree with this. We should have a default state that reflects an “out of the box” configuration, and if tests expects a different configuration, it should manage that within the context of the test.
-Sean On Tue, Mar 13, 2018 at 10:04 AM Kirk Lund <kl...@apache.org> wrote: > I want to propose using the default value for validate-serializable-object > in dunit tests instead of forcing it on for all dunit tests. I'm > sympathetic to the reason why this was done: ensure that all existing code > and future code will function properly with this feature enabled. > Unfortunately running all dunit tests with it turned on is not a good way > to achieve this. > > Here are my reasons: > > 1) All tests should start out with the same defaults that Users have. If we > don't do this, we are going to goof up sometime and release something that > only works with this feature turned on or worsen the User experience of > Geode in some way. > > 2) All tests should have sovereign control over their own configuration. We > should strive towards being able to look at a test and determine its config > at a glance without having to dig through the framework or other classes > for hidden configuration. We continue to improve dunit in this area but I > think adding to the problem is going in the wrong direction. > > 3) It sets a bad precedent. Do we follow this approach once or keep adding > additional non-default features when we need to test them too? Next one is > GEODE-4769 "Serialize region entry before putting in local cache" which > will be disabled by default in the next Geode release and yet by turning it > on by default for all of precheckin we were able to find lots of problems > in both the product code and test code. > > 4) This is already starting to cause confusion for developers thinking its > actually a product default or expecting it to be enabled in other > (non-dunit) tests. > > Alternatives for test coverage: > > There really are no reasonable shortcuts for end-to-end test coverage of > any feature. We need to write new tests or identify existing tests to > subclass with the feature enabled. > > 1) Subclass specific tests to turn validate-serializable-object on for that > test case. Examples of this include a) dunit tests that execute Region > tests with OffHeap enabled, b) dunit tests that flip on HTTP over GFSH, c) > dunit tests that run with SSL or additional security enabled. > > 2) Write new tests that cover all features with > validate-serializable-object > enabled. > > 3) Rerun all of dunit with and without the option. This doesn't sound very > reasonable to me, but it's the closest to what we really want or need. > > Any other ideas or suggestions other than forcing all dunit tests to run > with a non-default value? > > Thanks, > Kirk >
