Skip to content

Commit c68ba53

Browse files
aitorvsaitorvs
authored
Fix Android policy defaults and add Android tests (#3)
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1211947073192216?focus=true ### Description Update the Android library to use the proper policy defaults Incidentally also added Android specific tests and hooked them into CI pull requests ### Steps to test this PR Tests pass
1 parent 8c4821b commit c68ba53

File tree

10 files changed

+280
-3
lines changed

10 files changed

+280
-3
lines changed

‎.github/workflows/android-tests.yml‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
name: Android tests
2+
3+
on:
4+
push:
5+
paths:
6+
- "android/**"
7+
- "src/**"
8+
pull_request:
9+
paths:
10+
- "android/**"
11+
- "src/**"
12+
13+
jobs:
14+
android-tests:
15+
runs-on: ubuntu-latest
16+
17+
steps:
18+
- name: Checkout repository
19+
uses: actions/checkout@v4
20+
21+
# Rust is needed because host JNI lib is built for JVM tests
22+
- name: Install Rust toolchain
23+
uses: dtolnay/rust-toolchain@stable
24+
25+
- name: Set up JDK for Gradle
26+
uses: actions/setup-java@v4
27+
with:
28+
distribution: temurin
29+
java-version: '17'
30+
31+
- name: Cache Gradle
32+
uses: actions/cache@v4
33+
with:
34+
path: |
35+
~/.gradle/caches
36+
~/.gradle/wrapper
37+
key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}
38+
restore-keys: |
39+
${{ runner.os }}-gradle-
40+
41+
- name: Make scripts executable
42+
run: |
43+
chmod +x scripts/build_host_for_tests.sh
44+
cd android && chmod +x gradlew
45+
46+
# Option A: Let Gradle trigger the host build via the task you wired
47+
- name: Run JVM unit tests for ddg-url-predictor
48+
working-directory: android
49+
run: ./gradlew :ddg-url-predictor:testDebugUnitTest
50+

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ android/.idea/
3434
# (ignore if built outputs; keep if vendored)
3535
# =========================
3636
android/ddg-url-predictor/src/main/jniLibs/
37+
android/ddg-url-predictor/src/test/resources/
3738

3839
# =========================
3940
# Misc / OS

‎Cargo.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ serde = { version = "1", features = ["derive"] }
1414
serde_json = "1"
1515
once_cell = "1"
1616
publicsuffix = { version = "2", optional = true }
17+
jni = { version = "0.21", optional = true } # 👈 NEW
1718
[target.'cfg(target_os = "android")'.dependencies]
1819
jni = "0.21"
1920

@@ -22,3 +23,4 @@ jni = "0.21"
2223

2324
[features]
2425
real-psl = ["publicsuffix"]
26+
jni-host-tests = ["jni"]

‎PULL_REQUEST_TEMPLATE.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!--
2+
Note: This checklist is a reminder of our shared engineering expectations.
3+
The items in Bold are required
4+
If your PR involves UI changes:
5+
1. Upload screenshots or screencasts that illustrate the changes before / after
6+
2. Add them under the UI changes section (feel free to add more columns if needed)
7+
3. Make sure these changes are tested in API 23 and API 26
8+
If your PR does not involve UI changes, you can remove the **UI changes** section
9+
-->
10+
11+
Task/Issue URL:
12+
13+
### Description
14+
15+
### Steps to test this PR
16+
17+
_Feature 1_
18+
- [ ]
19+
- [ ]

‎README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,13 @@ Or run tests using the real PSL:
157157
cargo test --features real-psl
158158
```
159159

160+
To run the Android unit tests:
161+
```
162+
cargo build --release --features "real-psl jni-host-tests"
163+
cd android
164+
./gradlew :ddg-url-predictor:testDebugUnitTest
165+
```
166+
160167
---
161168

162169
## Notes

‎android/ddg-url-predictor/build.gradle‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,18 @@ dependencies {
100100
implementation platform("org.jetbrains.kotlin:kotlin-bom:1.9.24")
101101
implementation "org.jetbrains.kotlin:kotlin-stdlib" // version via BOM
102102
implementation "org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.3"
103-
}
103+
104+
testImplementation "org.jetbrains.kotlin:kotlin-test"
105+
testImplementation "junit:junit:4.13.2"
106+
}
107+
108+
// Task to run the host Rust build script for JVM tests
109+
tasks.register("buildHostNativeForTests", Exec) {
110+
workingDir "${projectDir}/../../scripts"
111+
commandLine "bash", "build_host_for_tests.sh"
112+
}
113+
114+
tasks.withType(Test).configureEach {
115+
dependsOn("buildHostNativeForTests")
116+
systemProperty "java.library.path", "${projectDir}/src/test/resources/jni"
117+
}

‎android/ddg-url-predictor/src/main/java/com/duckduckgo/urlpredictor/DecisionJson.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ object DecisionJson {
4040
"file",
4141
"about",
4242
"view-source",
43-
"mailto", "tel", "sms" // optional, if you want these to navigate
43+
"duck",
44+
"edge",
45+
"chrome",
4446
),
4547
)
4648
}

‎android/ddg-url-predictor/src/test/java/UrlPredictorTest.kt‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package com.duckduckgo.urlpredictor
2+
3+
import org.junit.Assert.*
4+
import org.junit.Test
5+
6+
class UrlPredictorTests {
7+
8+
private fun classify(input: String): Decision {
9+
return UrlPredictor.classify(input)
10+
}
11+
12+
// ------------------------------------------------------------------------
13+
// Basic behavior
14+
// ------------------------------------------------------------------------
15+
16+
@Test
17+
fun `single word becomes search`() {
18+
val d = classify("test")
19+
assertTrue(d is Decision.Search)
20+
assertEquals("test", (d as Decision.Search).query)
21+
}
22+
23+
@Test
24+
fun `simple http url navigates`() {
25+
val d = classify("http://example.com")
26+
assertTrue(d is Decision.Navigate)
27+
assertEquals("http://example.com/", (d as Decision.Navigate).url)
28+
}
29+
30+
// ------------------------------------------------------------------------
31+
// Telephone numbers (mirrors new Rust test)
32+
// ------------------------------------------------------------------------
33+
34+
@Test
35+
fun `telephone number plain digits is search`() {
36+
val d = classify("912345678")
37+
assertTrue(d is Decision.Search)
38+
assertEquals("912345678", (d as Decision.Search).query)
39+
}
40+
41+
@Test
42+
fun `telephone number with intl formatting is search`() {
43+
val input = "+351 912 345 678"
44+
val d = classify(input)
45+
assertTrue(d is Decision.Search)
46+
assertEquals(input, (d as Decision.Search).query)
47+
}
48+
49+
// ------------------------------------------------------------------------
50+
// Ports, userinfo, edge-cases (mirrors Rust tests)
51+
// ------------------------------------------------------------------------
52+
53+
@Test
54+
fun `host with port navigates`() {
55+
val d = classify("example.com:8080")
56+
assertTrue(d is Decision.Navigate)
57+
}
58+
59+
@Test
60+
fun `userinfo forces navigate`() {
61+
val d = classify("user:[email protected]")
62+
assertTrue(d is Decision.Navigate)
63+
}
64+
65+
// ------------------------------------------------------------------------
66+
// Schemes allowed vs disallowed
67+
// ------------------------------------------------------------------------
68+
69+
@Test
70+
fun `allowed scheme navigates`() {
71+
val d = classify("ftp://example.com")
72+
assertTrue(d is Decision.Navigate)
73+
}
74+
75+
// ------------------------------------------------------------------------
76+
// Single label intranet behaviors
77+
// ------------------------------------------------------------------------
78+
79+
@Test
80+
fun `single label host becomes search by default`() {
81+
val d = classify("intranet")
82+
assertTrue(d is Decision.Search)
83+
}
84+
85+
@Test
86+
fun `multi label intranet allowed when enabled`() {
87+
val d = classify("router.local")
88+
assertTrue(d is Decision.Navigate)
89+
}
90+
91+
// ------------------------------------------------------------------------
92+
// Unicode, invalid labels (mirrors Rust)
93+
// ------------------------------------------------------------------------
94+
95+
@Test
96+
fun `unicode IDNA host navigates`() {
97+
val d = classify("https://bücher.example")
98+
assertTrue(d is Decision.Navigate)
99+
}
100+
101+
@Test
102+
fun `invalid schemed-hostname navigates`() {
103+
val d = classify("http://-badlabel.com")
104+
assertTrue(d is Decision.Navigate)
105+
}
106+
107+
@Test
108+
fun `invalid bare-hostname becomes search`() {
109+
val d = classify("-badlabel.com")
110+
assertTrue(d is Decision.Search)
111+
}
112+
}

‎scripts/build_host_for_tests.sh‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
LIB_NAME="url_predictor"
5+
6+
# Resolve script dir (no matter where Gradle runs it from)
7+
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
8+
9+
# Rust crate root (one level above scripts/)
10+
RUST_ROOT="${SCRIPT_DIR}/.." # adjust if necessary
11+
RUST_TARGET_DIR="${RUST_ROOT}/target/release"
12+
13+
# Android module JVM test folder
14+
ANDROID_MODULE="${RUST_ROOT}/android/ddg-url-predictor"
15+
TEST_JNI="${ANDROID_MODULE}/src/test/resources/jni"
16+
17+
echo "SCRIPT_DIR = ${SCRIPT_DIR}"
18+
echo "RUST_ROOT = ${RUST_ROOT}"
19+
echo "RUST_TARGET_DIR = ${RUST_TARGET_DIR}"
20+
echo "TEST_JNI = ${TEST_JNI}"
21+
22+
echo "==> Building Rust library for HOST..."
23+
(
24+
cd "${RUST_ROOT}"
25+
cargo build --release --features "real-psl jni-host-tests"
26+
)
27+
28+
mkdir -p "${TEST_JNI}"
29+
30+
# Mac or Linux detection
31+
HOST_LIB=""
32+
if [[ "$OSTYPE" == "darwin"* ]]; then
33+
HOST_LIB="${RUST_TARGET_DIR}/lib${LIB_NAME}.dylib"
34+
elif [[ "$OSTYPE" == "linux"* ]]; then
35+
HOST_LIB="${RUST_TARGET_DIR}/lib${LIB_NAME}.so"
36+
else
37+
echo "Unsupported OS: $OSTYPE"
38+
exit 1
39+
fi
40+
41+
if [[ ! -f "$HOST_LIB" ]]; then
42+
echo "ERROR: Host library missing: $HOST_LIB"
43+
exit 1
44+
fi
45+
46+
echo "Copying $HOST_LIB → $TEST_JNI/"
47+
cp "$HOST_LIB" "$TEST_JNI"
48+
49+
echo "==> Done."
50+

‎src/lib.rs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ pub extern "C" fn ddg_up_get_psl_len() -> usize {
533533
// -----------------------------------------------------------------------------
534534
// JNI (Android only)
535535
// -----------------------------------------------------------------------------
536-
#[cfg(target_os = "android")]
536+
#[cfg(any(target_os = "android", feature = "jni-host-tests"))]
537537
#[no_mangle]
538538
pub extern "system" fn Java_com_duckduckgo_urlpredictor_UrlPredictor_ddgClassifyJni(
539539
mut env: jni::JNIEnv,
@@ -845,5 +845,25 @@ mod tests {
845845
assert!(matches!(classify("foo.local", &p), Decision::Navigate { .. }));
846846
assert!(matches!(classify("foo.localhost", &p), Decision::Navigate { .. }));
847847
}
848+
#[test]
849+
fn telephone_number_is_search() {
850+
let p = policy_default_inet();
851+
assert!(matches!(
852+
classify("tel:+123456789", &p),
853+
Decision::Search { query } if query == "tel:+123456789"
854+
));
855+
assert!(matches!(
856+
classify("tel:+4123423465", &p),
857+
Decision::Search { query } if query == "tel:+4123423465"
858+
));
859+
assert!(matches!(
860+
classify("912345678", &p),
861+
Decision::Search { query } if query == "912345678"
862+
));
863+
assert!(matches!(
864+
classify("+351 912 345 678", &p),
865+
Decision::Search { query } if query == "+351 912 345 678"
866+
));
867+
}
848868
}
849869

0 commit comments

Comments
 (0)