+0.5 I think we can turn this off (back to the default) now since the
AnalyzeSerializables tests for other modules have been created. These
tests ensure that serializable objects are properly white-listed or
excluded and are able to be serialized/deserialized.
Excluded classes are not tested, though, so if you add an excluded class
and are wrong about whether it is sent over the wire you will break
Geode. Having serialization validation enabled in dunit tests protects
against this if you write tests that use the excluded class.
We also have non-default values for cluster configuration in dunit runs.
On 3/13/18 10:03 AM, Kirk Lund 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
