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

mobile: Add initial unit tests for jni_utility.(h|cc) #30849

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

fredyw
Copy link
Member

@fredyw fredyw commented Nov 13, 2023

Prior to this PR, jni_utility.(h|cc) did not have any unit tests, which would make refactoring that file a bit risky. This PR sets up an initial infrastructure for writing unit tests for jni_utility.(h|cc) as well as adds an example unit test, so that we can slowly add unit tests for the functions defined in that file as well as requiring unit tests for future changes to that file.

Risk Level: low (adding unit tests only)
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: mobile

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #30849 was opened by fredyw.

see: more, trace.

This PR sets up an initial infrastructure for writing unit tests for
`jni_utility.(h|cc)`.

Signed-off-by: Fredy Wijaya <[email protected]>
@fredyw fredyw marked this pull request as ready for review November 13, 2023 17:18
@fredyw
Copy link
Member Author

fredyw commented Nov 13, 2023

/assign @abeyad

Copy link
Contributor

@abeyad abeyad left a comment

Choose a reason for hiding this comment

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

this is great!

// `test/java/io/envoyproxy/envoymobile/jni/JniUtilityTest.java` unit tests.

extern "C" JNIEXPORT jbyteArray JNICALL
Java_io_envoyproxy_envoymobile_jni_JniUtilityTest_protoJavaByteArrayConversion(JNIEnv* env, jclass,
Copy link
Contributor

Choose a reason for hiding this comment

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

so for every JNI method we want to test, we'll need a native test method in this file that does the similar to the function bodyhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. This is how we can call those functions from Java/Kotlin.

@abeyad abeyad merged commit 3c3372c into envoyproxy:main Nov 13, 2023
@fredyw fredyw deleted the jni_utility_test branch November 13, 2023 19:36
alyssawilk pushed a commit to alyssawilk/envoy that referenced this pull request Nov 13, 2023
This PR sets up an initial infrastructure for writing unit tests for
`jni_utility.(h|cc)`.

Signed-off-by: Fredy Wijaya <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants