Update robolectric to version 4.16 in parcelize-compiler #5649
Conversation
jaschdoc
requested review from
a team,
blueshiftlabs,
jsjeon,
ting-yuan and
troelsbjerre
as code owners
|
I see that there are some entries in |
| defaultDirectives { | ||
| +FIR_DIFFERENCE | ||
| +ENABLE_PARCELIZE | ||
| // Robolectric requires JDK 21 |
There was a problem hiding this comment.
Maybe mention the exact version (onward) of Roboelectric?
| @@ -1,5 +1,4 @@ | |||
| // WITH_STDLIB | |||
| // FULL_JDK | |||
There was a problem hiding this comment.
Is my understanding correct that now we need FULL_JDK for all tests since robolectric requires JDK 21? Having these redundant directives caused the issue? Well, actually my question is the other way around: how do we know these are only tests that have redundant directives?
There was a problem hiding this comment.
The FULL_JDK directive is directly not redundant, but results in an error now. However, you do raise a good point that maybe there are other redundant directives in the Parcelize tests. I will investigate a bit further and see if I can remove any directives.
A bit more context:
Both the TestJdkKind and JvmEnvironmentConfigurationDirectives (I will abbreviate this as JvmEnvConfigDir) classes define a FULL_JDK directive. It seems that JvmEnvConfigDir.FULL_JDK is not used for much other than:
- a (negative) toggle in ClassicFrontendFacade and
- to convert it to
TestJdkKind.FULL_JDKor throw an error if both directives are set inJvmEnvironmentConfigurator.
One conclusion from these observations is that JvmEnvConfigDir.FULL_JDK is almost redundant.
Additionally, according to the documentation, TestJdkKind.FULL_JDK is supposed to use the current JVM to run tests with, but in JvmBoxRunner, it is (mis-) used for looking up a JDK 8 in order to spawn a new JDK 8 process in which to run the tests.
I hope that provides a bit more insight. From this perspective, it seems like removing JvmEnvConfigDir.FULL_JDK is the best thing to do, but I can't at this point tell what other tests might break from doing so.
| } | ||
|
|
||
| with(visitAnnotation("Lorg/robolectric/annotation/Config;", true)) { | ||
| visit("sdk", intArrayOf(21)) |
There was a problem hiding this comment.
Ah, better to put a constant instead 👍🏼
jaschdoc
force-pushed
the
prr/jaschdoc/bump-robolectric-in-parcelize
branch
from
37df386 to
8a81629
Compare
jaschdoc
force-pushed
the
prr/jaschdoc/bump-robolectric-in-parcelize
branch
from
8a81629 to
f75fa23
Compare
jaschdoc
force-pushed
the
prr/jaschdoc/bump-robolectric-in-parcelize
branch
from
f75fa23 to
5b141d2
Compare
Also added new transitive dependencies, fixed classpath changes and updated outdated tests.
jaschdoc
force-pushed
the
prr/jaschdoc/bump-robolectric-in-parcelize
branch
from
5b141d2 to
d7ad107
Compare
Also added new transitive dependencies, fixed classpath changes and updated outdated tests. See https://issuetracker.google.com/467185160