Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(refactor): kotlinify layout property applicator test #41649

Conversation

MCanhisares
Copy link
Contributor

@MCanhisares MCanhisares commented Nov 27, 2023

Summary:

This PR converts to kotlin the java code for LayoutPropertyApplicatorTest, as requested in: #38825

Changelog:

[INTERNAL][CHANGED]: Convert LayoutPropertyApplicatorTest to Kotlin

Test Plan:

./gradlew :packages:react-native:ReactAndroid:test

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Nov 27, 2023
@MCanhisares MCanhisares force-pushed the refactor/layout-property-applicator-kotlin branch from 6194840 to d29fc21 Compare November 27, 2023 00:43
@MCanhisares MCanhisares force-pushed the refactor/layout-property-applicator-kotlin branch from d29fc21 to f8c2d2a Compare November 27, 2023 00:53
@MCanhisares
Copy link
Contributor Author

@cortinico @mdvacca
Following the discussion on #38825 , I've encountered the same issues reported by others in terms of mocking Yoga

I have successfully converted the tests to Kotlin, but wasn't able to figure out yet how to mock Yoga in order to instantiate LayoutShadowNode correctly; as such, I followed the same approach as this other PR from the umbrella issue and kept the @Ignore and that issue can be tackled in a further PR

Let me know if you have any ideas on how I can help solving this issue or any changes you want done in the tests

@MCanhisares MCanhisares changed the title (refactor): kotlinify layout proparty applicator test (refactor): kotlinify layout property applicator test Nov 27, 2023
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,645,046 -35,957
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,030,828 -33,192
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: b50a709
Branch: main

import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class)
@Ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried to remove this @Ignore and fix the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I have; I only added the @Ignore on 0ba9f2f to follow the approach suggested on #39284
The issues I'm facing are related to the issues reported on #38825, where Yoga is not mockable, which is a dependency for LayoutShadowNode. I tried using the ShadowSOLoader as you suggested on the thread, but it still didn't fix the issue, just changed the error stack to:

Could not initialize class com.facebook.react.uimanager.LayoutShadowNode
java.lang.NoClassDefFoundError: Could not initialize class com.facebook.react.uimanager.LayoutShadowNode
	at com.facebook.react.uimanager.LayoutPropertyApplicatorTest.testDimensions(LayoutPropertyApplicatorTest.kt:57)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:580)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:287)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.ExceptionInInitializerError: Exception java.lang.UnsatisfiedLinkError: 'long com.facebook.yoga.YogaNative.jni_YGConfigNewJNI()' [in thread "SDK 33 Main Thread"]
	at com.facebook.yoga.YogaNative.jni_YGConfigNewJNI(Native Method)
	at com.facebook.yoga.YogaConfigJNIBase.<init>(YogaConfigJNIBase.java:23)
	at com.facebook.yoga.YogaConfigJNIFinalizer.<init>(YogaConfigJNIFinalizer.java:12)
	at com.facebook.yoga.YogaConfigFactory.create(YogaConfigFactory.java:12)
	at com.facebook.react.uimanager.ReactYogaConfigProvider.get(ReactYogaConfigProvider.java:20)
	at com.facebook.react.uimanager.ReactShadowNodeImpl.<clinit>(ReactShadowNodeImpl.java:62)
	at com.facebook.react.uimanager.LayoutPropertyApplicatorTest.testPadding(LayoutPropertyApplicatorTest.kt:195)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the SoLoader as a config for the test class, the error stack is:

no yoga in java.library.path: /Users/marcelcanhisares/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:/System/Library/Java/Extensions:/usr/lib/java:.
java.lang.UnsatisfiedLinkError: no yoga in java.library.path: /Users/marcelcanhisares/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:/System/Library/Java/Extensions:/usr/lib/java:.
	at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2434)
	at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:818)
	at java.base/java.lang.System.loadLibrary(System.java:1989)
	at com.facebook.soloader.SoLoader.loadLibraryOnNonAndroid(SoLoader.java:926)
	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:882)
	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:869)
	at com.facebook.yoga.YogaNative.<clinit>(YogaNative.java:16)
	at com.facebook.yoga.YogaConfigJNIBase.<init>(YogaConfigJNIBase.java:23)
	at com.facebook.yoga.YogaConfigJNIFinalizer.<init>(YogaConfigJNIFinalizer.java:12)
	at com.facebook.yoga.YogaConfigFactory.create(YogaConfigFactory.java:12)
	at com.facebook.react.uimanager.ReactYogaConfigProvider.get(ReactYogaConfigProvider.java:20)
	at com.facebook.react.uimanager.ReactShadowNodeImpl.<clinit>(ReactShadowNodeImpl.java:62)
	at com.facebook.react.uimanager.LayoutPropertyApplicatorTest.testDimensions(LayoutPropertyApplicatorTest.kt:55)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:580)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:287)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I have; I only added the @ignore on 0ba9f2f to follow the approach suggested on #39284

Ah I see. Ok let's merge it as is and I'll do a pass afterwards in trying to fix this and other suppressed tests.

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link

This pull request was successfully merged by @MCanhisares in 3654089.

When will my fix make it into a release? | Upcoming Releases

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 29, 2023
@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in 3654089.

@MCanhisares MCanhisares deleted the refactor/layout-property-applicator-kotlin branch December 1, 2023 16:57
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
Summary:
This PR converts to kotlin the java code for LayoutPropertyApplicatorTest, as requested in: facebook#38825

## Changelog:

[INTERNAL][CHANGED]: Convert LayoutPropertyApplicatorTest to Kotlin

Pull Request resolved: facebook#41649

Test Plan: `./gradlew :packages:react-native:ReactAndroid:test `

Reviewed By: rshest

Differential Revision: D51614921

Pulled By: cortinico

fbshipit-source-id: 06ee403a496f34afc9abeabc0c406391e316538a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants