truth
truth copied to clipboard
isInstanceOf/isNotInstanceOf can't find supertypes under GWT (if class metadata is off?)
If I adopt isInstanceOf
in Guava, various tests fail. These are tests that worked fine with assertTrue(foo instanceof Bar)
. Truth has special support for isInstanceOf
under GWT, but it can't always work: My understanding is that getSuperclass()
is unavailable if -XdisableClassMetadata
is set:
https://code.google.com/p/google-web-toolkit/wiki/NoClassMetadataOptimization https://code.google.com/p/google-web-toolkit/source/detail?r=4790
There are various directions we could go here. We can fail isInstanceOf
and/or isNotInstanceOf
calls under GWT, whether unconditionally or only if class metadata is absent. We could fail some in one set of circumstances and some in the other. If we try to do our best without class metadata, we could add some warnings to the failure message. Or we can remove the methods entirely from GWT. (But that seems heavy handed, since I expect few users to run tests with class metadata off. Speaking of which, why not just turn on class metadata in Guava's tests? Our goal is to make sure that our prod code functions correctly with it off. This has the unfortunate effect of forcing our test code to do the same.)
I suggest something like this:
-
isNotInstanceOf
fails under-XdisableClassMetadata
, since we it can't do its job unless it's checking that the class is exactly==
to another -
isInstanceOf
either does the same thing, or it tries to check==
and then fails with a warning that it might be failing wrongly
(Also, we should expand our tests that these methods work correctly with class metadata on. We have some tests already, but I don't think we currently exercise isInstanceOf(SomeInterface.class)
.)
A problem with my suggested plan: It works fine for assert/expect, but it does the wrong thing for assume, where the way of producing a falsely passing test is reversed.
We could try to special case assume, but that won't help with a custom FailureStrategy
. My new suggestion would be for isInstanceOf
to either fail always or to fail whenever the classes aren't ==
. And by "fail," I mean "throw UnsupportedOperationException
", not "call fail()
".
My 2 cents: these methods should be banned in GWT-compatible code until a better solution can be offered - isNotInstanceOf() will succeed erroneously, which is a serious problem.
You have shamed me into trying to do the easiest thing that might plug the biggest hole -- making isNotInstanceOf
throw if class metadata is unavailable :) I'll report back.
I forget the exact history here, but I think I left some things undone:
- Run tests with class metadata off (for both j2cl and old-style GWT).
- I think j2cl and old-style GWT may behave slightly differently. I thought I had left some notes on this somewhere, but I don't see them. Presumably the tests would find the problems again, at which point we should fix them.
- We could still consider removing
isInstanceOf
and especiallyisNotInstanceOf
entirely under GWT/j2cl, since they have significant limitations.