diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ae3975d..ff9d095 100644 --- a/.gitlab-ci.yml +++ b/.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 diff --git a/app/src/androidTest/kotlin/io/flamenet/messenger/app/SignUpFlowTest.kt b/app/src/androidTest/kotlin/io/flamenet/messenger/app/SignUpFlowTest.kt index c6944ab..9066376 100644 --- a/app/src/androidTest/kotlin/io/flamenet/messenger/app/SignUpFlowTest.kt +++ b/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() } diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 0d736ae..47cbd49 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/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