Merge branch 'fix/instrumented-clears-a-release-build' into 'main'

The on-device sign-up suite runs again, and the bug it found is fixed

Closes #22

See merge request paulhitt/flamenet-messenger-android!122

.gitlab-ci.yml

@@ -77,6 +77,21 @@ unit-tests:
     # survives, and a suite that stopped running entirely would still be
     # counted, by the very check meant to notice that it had stopped.
     - rm -rf ./*/build/test-results
+    # Keep the build output out of Spotlight.
+    #
+    # The module build directories are rewritten every job on a host that is
+    # also the mac-local runner for both app repos. Indexing them drives the
+    # machine to a load average in the hundreds, with `mds_stores` and
+    # `mdworker_shared` at the top of CPU while no build process is near it.
+    # Measured on the iPhone repo: the same from-scratch build peaks at load
+    # ~12 with its output excluded and well over a hundred without.
+    #
+    # Written here rather than committed, because the directories these belong
+    # in are created by the build.
+    - |
+      for m in app engine client obfs4; do
+        mkdir -p "$m/build" && touch "$m/build/.metadata_never_index"
+      done
     # `:obfs4:test` was missing from this list. The module shipped in !77
     # with ~100 tests, the Elligator2 port, the framing, the handshake, the
     # socket factory, and not one of them ran in CI. A break in the transport

app/src/androidTest/kotlin/io/flamenet/messenger/app/SignUpFlowTest.kt

@@ -6,6 +6,7 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule
 import androidx.compose.ui.test.onNodeWithTag
 import androidx.compose.ui.test.onRoot
 import androidx.compose.ui.test.performClick
+import androidx.compose.ui.test.performScrollTo
 import androidx.compose.ui.test.performTextClearance
 import androidx.compose.ui.test.performTextInput
 import androidx.compose.ui.test.printToString
@@ -77,7 +78,12 @@ class SignUpFlowTest {
         if (nodesWithTag("nav-account").isEmpty()) return
         compose.onNodeWithTag("nav-account").performClick()
         awaitTag("sign-out", 20_000)
-        compose.onNodeWithTag("sign-out").performClick()
+        // Scrolled to, for the second time in this file and the same reason.
+        // Sign out is the last section of a long Account screen, under the
+        // profile picture, the status list and the version, so on a phone it is
+        // well below the fold. The tap was going nowhere and the wait that
+        // followed then blamed the sign-in form for not appearing.
+        compose.onNodeWithTag("sign-out").performScrollTo().performClick()
         awaitTag("relay", 30_000)
     }
 
@@ -152,24 +158,75 @@ class SignUpFlowTest {
 
         compose.onNodeWithTag("relay").performTextClearance()
         compose.onNodeWithTag("relay").performTextInput(relay)
-        compose.onNodeWithTag("toggle-register").performClick()
+
+        // Wait for the relay probe to land BEFORE switching to the register
+        // form, and that order is the whole fix.
+        //
+        // `registering` is a `remember` on the sign-in screen. The probe
+        // finishing recomposes that screen, the remember goes back to its
+        // initial `false`, and a tap that happened while the probe was still in
+        // flight is undone. The test then sat on the sign-in form waiting for an
+        // email field that only the register form has, and reported it as
+        // missing, which reads like the field was deleted from the app.
+        //
+        // `identity-mode` is rendered from `site?.let`, so it appears exactly
+        // when the probe has answered and not before.
+        awaitTag("identity-mode", 30_000)
+        // Scrolled to before it is tapped. The sign-in form is inside a
+        // vertical scroller and this button sits below the fold on a phone-sized
+        // screen, which the failure dump showed as nodes at negative y. A tap
+        // aimed off-screen is not an error in Compose, it simply does not
+        // happen, so the form stayed on sign-in and the test then waited thirty
+        // seconds for a field that only appears after the switch.
+        compose.onNodeWithTag("toggle-register").performScrollTo().performClick()
 
         // A relay with its own account table asks for an email; the shared
-        // service does not.
+        // service does not. Waited for rather than asserted outright, because
+        // whether this relay is one of those is something the probe above
+        // decides.
+        awaitTag("email", 30_000)
         compose.onNodeWithTag("email").assertIsDisplayed()
-        compose.onNodeWithTag("email").performTextInput("$handle@example.invalid")
-        compose.onNodeWithTag("handle").performTextInput(handle)
-        compose.onNodeWithTag("passphrase").performTextInput("a-long-enough-passphrase")
+
+        // Cleared before each is typed, not just the relay field.
+        //
+        // `performTextInput` APPENDS. Signing out leaves the form filled in
+        // with what was typed last time, so the second test in the class was
+        // producing "19901990" in the age gate. Submit is enabled only on
+        // exactly four digits, a disabled button swallows a tap without
+        // complaint, and the run then waited two minutes for a screen the tap
+        // was never going to reach. The same shape as the age gate bug this
+        // suite was fixed for once already.
+        for (field in listOf("email", "handle", "passphrase", "birth-year")) {
+            compose.onNodeWithTag(field).performScrollTo().performTextClearance()
+        }
+
+        compose.onNodeWithTag("email").performScrollTo().performTextInput("$handle@example.invalid")
+        compose.onNodeWithTag("handle").performScrollTo().performTextInput(handle)
+        compose.onNodeWithTag("passphrase").performScrollTo().performTextInput("a-long-enough-passphrase")
         // Submit is disabled until the age gate has four digits, and
         // performClick on a disabled node does NOT throw: it does nothing, and
         // the run then fails two minutes later looking for a screen the tap
         // never reached.
-        compose.onNodeWithTag("birth-year").performTextInput("1990")
-        compose.onNodeWithTag("submit").performClick()
+        compose.onNodeWithTag("birth-year").performScrollTo().performTextInput("1990")
+        compose.onNodeWithTag("submit").performScrollTo().performClick()
 
         // Generous: sign-up is a network round trip plus a device registration
         // publishing twenty prekeys, on an emulator.
-        awaitTag("device-id", 120_000)
+        //
+        // Signing up lands on Home, and `device-id` is on the Account screen,
+        // so waiting for it here waited two minutes on a screen that was never
+        // going to show it and then reported the app had not signed in. It had:
+        // the failure dump said "Nobody here yet. Add someone by the username
+        // they signed up with", which is the empty contact list of an account
+        // that had just been created.
+        //
+        // The nav pill is the honest signal that the shell is up, and the
+        // device id is still checked, on the screen that has it, because a
+        // registered device is the half of sign-up that a landing screen alone
+        // does not prove.
+        awaitTag("nav-account", 120_000)
+        compose.onNodeWithTag("nav-account").performClick()
+        awaitTag("device-id", 30_000)
         compose.onNodeWithTag("device-id").assertIsDisplayed()
     }
 

app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt

@@ -515,11 +515,33 @@ class MessengerViewModel(
             }
 
         val vault = vault ?: error("the vault is not open")
+
         // The stored identity FIRST, and this line is why signing out and back
-        // in used to cost every contact a prompt.
+        // in used to cost every contact a prompt: keeping the device means
+        // keeping every session built on it.
+        //
+        // Only when it is the SAME account, though. Signing out plainly keeps
+        // the device on purpose, so a second sign-up on the same phone used to
+        // reuse a device that already belonged to somebody else and try to
+        // register it against the new account. The relay refused, correctly,
+        // and the refusal surfaced to the person as "Your session expired.
+        // Sign out and back in to keep going" on the sign-up screen, which is
+        // advice nobody can act on: they had already signed out, and that IS
+        // the screen you sign up from. See android#22.
+        //
+        // Same relay and same user id means the device is ours to reuse.
+        // Anything else is a different account on a shared phone, and it gets
+        // its own device, which is what "a device belongs to an account" means.
+        val previous = runCatching { vault.loadCredential() }.getOrNull()
+        val sameAccount =
+            previous == null ||
+                (previous.userId == account.userId && previous.relayBaseUrl == config.baseUrl)
+        val storedIdentity = if (sameAccount) vault.loadIdentity() else null
+        if (!sameAccount) identity = null
+
         val device =
             identity
-                ?: vault.loadIdentity()?.also { identity = it }
+                ?: storedIdentity?.also { identity = it }
                 ?: DeviceIdentity.create().also {
                     vault.saveIdentity(it)
                     identity = it