diff --git a/.github/workflows/android-sanity-check.yml b/.github/workflows/android-sanity-check.yml index ae8152c..25a5f53 100644 --- a/.github/workflows/android-sanity-check.yml +++ b/.github/workflows/android-sanity-check.yml @@ -31,8 +31,16 @@ jobs: - name: Grant Execute Permission to Gradlew run: chmod +x gradlew -# - name: Run Android Lint (Syntax & Rules Check) -# run: ./gradlew lintDebug + # Phase 0 safety net: unit tests are a BLOCKING gate. + - name: Run Unit Tests + run: ./gradlew testDebugUnitTest --stacktrace + + # Lint runs for visibility only and never blocks the job (continue-on-error). + # Scoped to :app so the vendored termux-core/upstream lint backlog does not fail CI. + # Once the :app lint backlog is triaged, set abortOnError=true and drop continue-on-error. + - name: Run Android Lint (reporting, non-blocking) + continue-on-error: true + run: ./gradlew :app:lintDebug - name: Compile Test (Assemble Debug) run: ./gradlew assembleDebug --stacktrace diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..ab4f57a --- /dev/null +++ b/.gitignore @@ -0,0 +1,9 @@ +# IDE / editor +.idea/ +*.iml +*.iws +*.ipr + +# OS +.DS_Store +Thumbs.db diff --git a/controller/app/build.gradle b/controller/app/build.gradle index 3c68339..6f50c6e 100644 --- a/controller/app/build.gradle +++ b/controller/app/build.gradle @@ -105,6 +105,14 @@ android { abortOnError false } + testOptions { + unitTests { + // Let JVM unit tests stub Android framework calls (e.g. android.util.Log) + // so framework-free logic can be tested without an emulator. + returnDefaultValues = true + } + } + dependenciesInfo { includeInApk = false includeInBundle = false @@ -149,6 +157,8 @@ dependencies { // Testing testImplementation 'junit:junit:4.13.2' + // Real org.json so SyncHandshakeHelper JSON logic can run in JVM unit tests + testImplementation 'org.json:json:20231013' androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1' androidTestImplementation 'androidx.test.ext:junit:1.1.5' } diff --git a/controller/app/src/main/java/org/iiab/controller/DashboardFragment.java b/controller/app/src/main/java/org/iiab/controller/DashboardFragment.java index d842109..980bf5c 100644 --- a/controller/app/src/main/java/org/iiab/controller/DashboardFragment.java +++ b/controller/app/src/main/java/org/iiab/controller/DashboardFragment.java @@ -567,12 +567,7 @@ private boolean pingUrl(String urlStr) { // Extracts the numbers (in kB) from the lines of /proc/meminfo private long parseMemLine(String line) { - try { - String[] parts = line.split("\\s+"); - return Long.parseLong(parts[1]); - } catch (Exception e) { - return 0; - } + return SystemStatsUtil.parseMemLine(line); } // --- METHODS FOR OBTAINING IPs --- @@ -719,13 +714,7 @@ private String getTermuxArch() { } private String getDebianArch(String androidArch) { - if (androidArch == null || androidArch.equals("N/A")) return "N/A"; - String lower = androidArch.toLowerCase(); - - if (lower.contains("arm64") || lower.contains("aarch64")) return "arm64"; - if (lower.contains("armeabi") || lower.contains("armv7")) return "armhf"; - - return lower; + return SystemStatsUtil.getDebianArch(androidArch); } // Converter from DP to actual screen pixels diff --git a/controller/app/src/main/java/org/iiab/controller/SystemStatsUtil.java b/controller/app/src/main/java/org/iiab/controller/SystemStatsUtil.java new file mode 100644 index 0000000..f1e3b80 --- /dev/null +++ b/controller/app/src/main/java/org/iiab/controller/SystemStatsUtil.java @@ -0,0 +1,60 @@ +package org.iiab.controller; + +/** + * Framework-free helpers for parsing system statistics. + * + *
These functions were extracted from {@code DashboardFragment} so they can be + * unit-tested on the plain JVM (no Android dependencies, no emulator). Keep this + * class free of any {@code android.*} imports. + */ +public final class SystemStatsUtil { + + private SystemStatsUtil() { + // Utility class — no instances. + } + + /** + * Parses a single {@code /proc/meminfo} line and returns the numeric value (in kB). + * + *
A meminfo line looks like {@code "MemTotal: 8127200 kB"}. The value is the + * second whitespace-separated token. Returns {@code 0} for any malformed or null input + * rather than throwing, preserving the original defensive behavior. + * + * @param line a line from {@code /proc/meminfo}, e.g. {@code "MemAvailable: 123456 kB"} + * @return the parsed value in kB, or {@code 0} if the line cannot be parsed + */ + public static long parseMemLine(String line) { + if (line == null) { + return 0; + } + try { + String[] parts = line.trim().split("\\s+"); + return Long.parseLong(parts[1]); + } catch (Exception e) { + return 0; + } + } + + /** + * Maps an Android/Termux architecture string to the matching Debian architecture name. + * + * @param androidArch the Android architecture (e.g. {@code "aarch64"}, {@code "armv7l"}) + * @return the Debian architecture ({@code "arm64"}, {@code "armhf"}), {@code "N/A"} when + * unknown/empty, or the lower-cased input when no mapping applies + */ + public static String getDebianArch(String androidArch) { + if (androidArch == null || androidArch.equals("N/A")) { + return "N/A"; + } + String lower = androidArch.toLowerCase(); + + if (lower.contains("arm64") || lower.contains("aarch64")) { + return "arm64"; + } + if (lower.contains("armeabi") || lower.contains("armv7")) { + return "armhf"; + } + + return lower; + } +} diff --git a/controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java b/controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java new file mode 100644 index 0000000..b2ac4bd --- /dev/null +++ b/controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java @@ -0,0 +1,82 @@ +package org.iiab.controller; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +/** + * Unit tests for the pure (framework-free) parts of {@link SyncHandshakeHelper}: + * password generation and the QR payload create/parse round-trip. + * + *
Relies on {@code testOptions.unitTests.returnDefaultValues = true} (so + * {@code android.util.Log} calls no-op) and the real {@code org.json} test dependency. + */ +public class SyncHandshakeHelperTest { + + // --- generateSecurePassword --- + + @Test + public void generateSecurePassword_hasExpectedLength() { + assertEquals(12, SyncHandshakeHelper.generateSecurePassword().length()); + } + + @Test + public void generateSecurePassword_usesOnlyAlphanumericChars() { + String pwd = SyncHandshakeHelper.generateSecurePassword(); + assertTrue("password should be alphanumeric: " + pwd, pwd.matches("[A-Za-z0-9]+")); + } + + @Test + public void generateSecurePassword_isNotConstant() { + // Extremely unlikely to collide; guards against a degenerate generator. + assertTrue(!SyncHandshakeHelper.generateSecurePassword() + .equals(SyncHandshakeHelper.generateSecurePassword())); + } + + // --- createPayload / parsePayload round-trip --- + + @Test + public void payload_roundTripsAllFields() { + String payload = SyncHandshakeHelper.createPayload( + "192.168.1.50", 8730, "iiab_peer", "s3cretPass", true, 64); + + SyncHandshakeHelper.SyncCredentials creds = SyncHandshakeHelper.parsePayload(payload); + + assertNotNull(creds); + assertEquals("192.168.1.50", creds.ip); + assertEquals(8730, creds.port); + assertEquals("iiab_peer", creds.user); + assertEquals("s3cretPass", creds.pass); + assertTrue(creds.hasRootfs); + assertEquals(64, creds.archBits); + } + + @Test + public void parsePayload_returnsNullForNonIiabJson() { + assertNull(SyncHandshakeHelper.parsePayload("{\"app\":\"some_other_app\"}")); + } + + @Test + public void parsePayload_returnsNullForMalformedJson() { + assertNull(SyncHandshakeHelper.parsePayload("this is not json")); + } + + @Test + public void parsePayload_returnsNullForNull() { + assertNull(SyncHandshakeHelper.parsePayload(null)); + } + + @Test + public void parsePayload_defaultsHasRootfsTrueForLegacyPayload() { + // Legacy payloads without "has_rootfs" should default to true. + String legacy = "{\"app\":\"iiab_sync\",\"ip\":\"10.0.0.1\",\"port\":8730," + + "\"user\":\"u\",\"pass\":\"p\"}"; + SyncHandshakeHelper.SyncCredentials creds = SyncHandshakeHelper.parsePayload(legacy); + assertNotNull(creds); + assertTrue(creds.hasRootfs); + assertEquals(0, creds.archBits); + } +} diff --git a/controller/app/src/test/java/org/iiab/controller/SystemStatsUtilTest.java b/controller/app/src/test/java/org/iiab/controller/SystemStatsUtilTest.java new file mode 100644 index 0000000..064e2d9 --- /dev/null +++ b/controller/app/src/test/java/org/iiab/controller/SystemStatsUtilTest.java @@ -0,0 +1,86 @@ +package org.iiab.controller; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +/** + * Unit tests for {@link SystemStatsUtil}. These run on the plain JVM — no emulator. + * This is the first safety-net test added in Phase 0 of the tech-debt remediation plan. + */ +public class SystemStatsUtilTest { + + // --- parseMemLine --- + + @Test + public void parseMemLine_parsesStandardMeminfoLine() { + assertEquals(8127200L, SystemStatsUtil.parseMemLine("MemTotal: 8127200 kB")); + } + + @Test + public void parseMemLine_parsesAvailableLine() { + assertEquals(123456L, SystemStatsUtil.parseMemLine("MemAvailable: 123456 kB")); + } + + @Test + public void parseMemLine_handlesLeadingAndTrailingWhitespace() { + assertEquals(42L, SystemStatsUtil.parseMemLine(" SwapFree: 42 kB ")); + } + + @Test + public void parseMemLine_returnsZeroForNull() { + assertEquals(0L, SystemStatsUtil.parseMemLine(null)); + } + + @Test + public void parseMemLine_returnsZeroForEmpty() { + assertEquals(0L, SystemStatsUtil.parseMemLine("")); + } + + @Test + public void parseMemLine_returnsZeroWhenSecondTokenNotNumeric() { + assertEquals(0L, SystemStatsUtil.parseMemLine("MemTotal: notANumber kB")); + } + + @Test + public void parseMemLine_returnsZeroWhenNoSecondToken() { + assertEquals(0L, SystemStatsUtil.parseMemLine("MemTotal:")); + } + + // --- getDebianArch --- + + @Test + public void getDebianArch_mapsAarch64ToArm64() { + assertEquals("arm64", SystemStatsUtil.getDebianArch("aarch64")); + } + + @Test + public void getDebianArch_mapsArm64VariantToArm64() { + assertEquals("arm64", SystemStatsUtil.getDebianArch("ARM64-v8a")); + } + + @Test + public void getDebianArch_mapsArmeabiToArmhf() { + assertEquals("armhf", SystemStatsUtil.getDebianArch("armeabi-v7a")); + } + + @Test + public void getDebianArch_mapsArmv7ToArmhf() { + assertEquals("armhf", SystemStatsUtil.getDebianArch("armv7l")); + } + + @Test + public void getDebianArch_returnsNaForNull() { + assertEquals("N/A", SystemStatsUtil.getDebianArch(null)); + } + + @Test + public void getDebianArch_returnsNaForNaSentinel() { + assertEquals("N/A", SystemStatsUtil.getDebianArch("N/A")); + } + + @Test + public void getDebianArch_lowercasesUnknownArch() { + assertEquals("x86_64", SystemStatsUtil.getDebianArch("X86_64")); + } +} diff --git a/controller/docs/FORK_DELTA_ANALYSIS.md b/controller/docs/FORK_DELTA_ANALYSIS.md new file mode 100644 index 0000000..53ecb1d --- /dev/null +++ b/controller/docs/FORK_DELTA_ANALYSIS.md @@ -0,0 +1,44 @@ +# Termux Fork — Delta Analysis (iiab changes vs upstream) + +> Scope: technical debt **only in what iiab modified** in the vendored Termux fork at `controller/termux-core/termux-source` (submodule → `github.com/iiab/termux-app`). Upstream code is out of scope. Date: 2026-06-16. + +## 1. What iiab actually changed + +The fork's HEAD (`f8f36614`) sits **8 commits** above upstream base `30ebb2de`, all authored by `Ark74`. The **net delta is a single method** — `ExtraKeysView.loadIIABDefaultKeys()`, +25 lines in: + +``` +termux-shared/src/main/java/com/termux/shared/termux/extrakeys/ExtraKeysView.java +``` + +It builds a hardcoded extra-keys layout and calls the existing public `reload(ExtraKeysInfo, float)`. It is called once, from `controller/app/.../MainActivity.java:2036`, right before wiring the extra-keys click listener. + +The 8 commits are: a 44-line feature commit, a "decouple" commit that removed 24 lines, and six small "fix: yet another change pt2…pt5" tweaks that net to the final 25-line method. + +## 2. Findings (scoped to the delta) + +Scoring: **Priority = (Impact + Risk) × (6 − Effort)**, each 1–5. + +| ID | Location | Category | Issue | Imp | Risk | Eff | Prio | +|----|----------|----------|-------|----|----|----|----| +| K1 | `ExtraKeysView.java:682–706` | Architecture / fork maintenance | The change lives **inside an upstream file** but uses only public APIs (`reload`, public `ExtraKeysInfo` ctor, public `ExtraKeyDisplayMap`). The same result is achievable entirely from the controller app — so the fork need not modify upstream source at all. Every upstream sync will now conflict on this file. | 4 | 3 | 2 | 28 | +| K2 | 8 commits `e6c7b88d..f8f36614` | Documentation / process | Unreviewable history: five commits named `fix: yet another change pt2…pt5` with no body, netting 25 lines. Can't bisect, cherry-pick, or review intent. | 3 | 2 | 1 | 25 | +| K3 | `ExtraKeysView.java:688–692` | Code | Keyboard layout is a **hardcoded inline pseudo-JSON string** in Java; the comment "we match the same Termux keys" admits manual drift. Duplicates Termux's normal properties-driven layout and the controller's own ESC/TAB/… key-handling switch (`MainActivity.java:~2040+`). | 2 | 2 | 2 | 16 | +| K4 | `ExtraKeysView.java:702–704` | Code | Broad `catch (Exception e)` logs and continues; if layout construction fails the user silently gets **no extra keys** with no fallback to upstream defaults. | 2 | 2 | 1 | 20 | +| K5 | `ExtraKeysView.java:686` (no test) | Test | The layout string's validity (parses into a valid `ExtraKeysInfo`) is untested. Once the literal moves into the app as a constant, it becomes a trivial JVM unit test. | 2 | 2 | 2 | 16 | +| K6 | `ExtraKeysView.java:682–686, 703` | Code (cosmetic) | Malformed Javadoc (`/**` at column 0 while body is indented), decorative `===` banner comments, and fully-qualified `android.util.Log` instead of an import. The inline-FQN/no-import-change is actually a reasonable merge-conflict-minimization tactic — only matters once the code relocates. | 1 | 1 | 1 | 10 | + +## 3. Top recommendation — relocate the change out of upstream (K1) + +This single move resolves the core maintainability problem and directly supports the submodule/build work just completed. + +1. Move the body of `loadIIABDefaultKeys()` into the controller app — e.g. a small helper that builds the `ExtraKeysInfo` (layout as a named constant or `R.string`/resource, addressing **K3**) and calls `extraKeysView.reload(iiabKeysInfo, 0f)` directly. `MainActivity.java:2036` already holds the `extraKeysView` reference, so the call site barely changes. +2. Revert `ExtraKeysView.java` to its upstream contents and **pin the submodule to a clean upstream tag**. The fork becomes a pristine mirror → upstream upgrades stop conflicting. +3. Add a fallback (K4): if the custom layout fails to build, fall back to Termux's default keys rather than showing none. +4. Squash the 8 commits into one well-described commit before any further sharing (**K2**). +5. Add the layout-validity unit test once the constant lives in the app (**K5**), fitting the Phase 0 safety-net pattern. + +Net effect: **zero iiab modifications to vendored upstream**, a single-source-of-truth layout shared with the controller's key handling, and a fork that tracks upstream cleanly. + +## 4. Note + +The earlier `controller/app` analysis (`TECH_DEBT_PLAN.md`) is unaffected by the submodule now being present — those 34 files are unchanged. The submodule only matters for building (dependency resolution), which is handled on-device via Android Studio and in CI.