Session Pro on Desktop: the same fifteen specs, and three mobile specs that were not asserting - #127
Open
mpretty-cyro wants to merge 5 commits into
Open
Session Pro on Desktop: the same fifteen specs, and three mobile specs that were not asserting#127mpretty-cyro wants to merge 5 commits into
mpretty-cyro wants to merge 5 commits into
Conversation
makeAccountPro mints against the Pro backend rather than driving a device, so it was already platform-neutral - but it lived in run/test/utils/, alongside open_app and capabilities_ios, where a Desktop import reads as reaching into the mobile harness. Moved to run/shared/, next to the other platform-neutral helpers, and decoupled from the two mobile types it referenced: the user parameter becomes a structural ProAccountUnderTest that mobile's User satisfies as-is, and the platform union is inlined. The module now imports nothing from run/test/utils/, so Desktop consuming it is not a layering violation.
…erver Four pieces the Desktop Pro specs need, none of which existed: Pro mocks (run/desktop/pro_mocks.ts) translate the shared ProMockContext vocabulary into the env session-desktop reads, so the same two fields mean the same thing in a Desktop spec as in a bothPlatformsIt one. SESSION_PRO is implied by asking for a Pro state at all - forgetting it renders an empty screen rather than erroring. The run's ambient SESSION_PRO is preserved when a test asks for no Pro context, so the pre-existing message-length cases keep taking availability from the run. The @Pro tag is derived from that context rather than declared per test, so it cannot disagree with what the spec actually sets up - mobile's hand-written isPro can. Restarts (run/desktop/restart.ts) relaunch a window on the same user-data directory. Session Desktop asks the Pro backend for status once, at startup, so a grant made while the app is running is invisible until it comes back up. The old process must be killed first: Electron's single-instance lock means a second launch against the same NODE_APP_INSTANCE exits instead of opening a window, which surfaces as firstWindow timing out rather than as a launch error. It lives outside DesktopWrapper to keep the invariant that the wrapper never launches or kills Electron. The local file server is mapped from the harness's own FILE_SERVER_URL plus a new FILE_SERVER_ED_PUBKEY. Two keys for one server and they are not interchangeable: the mobile clients need the X25519 key for onion encryption, Desktop needs the Ed25519 one because it embeds it in the returned URL for the download leg to re-derive from. Both are 64 hex characters, so the wrong one fails no format check. communityRooms is plumbed through to match sessionIt, so getCommunities() behaves the same on both platforms against a local SOGS.
Adds the Pro settings locators, the badge toggle, the edit-profile-picture Pro badge and the picked- image preview, plus the verbs the specs drive them with: subscribeToPro, waitForProActive, enableProBadge, uploadProfilePicture. Two things here are load-bearing rather than incidental: CTA feature rows render their bullet as an icon-font glyph inside the same text node as the copy, so innerText carries a private-use character. Stripping that range lets the shared cross-platform CTA table be compared verbatim - without it the failure is unreadable, because the glyph has no width in a terminal and expected and actual print identically. A CTA with only a negative button is legitimate - an acknowledgement rather than an offer - but checkCTAStrings reads buttons positionally, so collapsing them asserted Close against a confirm button that does not exist. The uploadProfilePicture flow waits for the picked image's preview, not for the Save button: Save reports itself enabled before the image is processed and handleUpload then returns silently on !avatarChanged, so an early click is swallowed and nothing clicks again. Telling the upload path from the upsell-CTA path is a race between the two outcomes rather than a fixed wait, which would otherwise cost its full duration on every Pro account.
The same fifteen the mobile suite runs: the four Pro settings states, both pinned-conversation limits, the four Pro message lengths, the three animated display picture cases and the Pro badge a second client verifies. Each spec asserts the ABSENCE of the other state's rows as well as the presence of its own. That is what distinguishes the screens - presence alone would pass an app rendering both - and the same applies to the pin limits, which assert the pin actually took rather than only that no CTA appeared. Three notes on where Desktop genuinely differs: There is no app-open expiry CTA here. handleTriggeredCTAs returns early when fromAppStart is true and fires off a stored flag, so no status mock can produce it. Raised as a product divergence rather than worked around. The Pro settings status banner renders in every state, empty on success, so it is matched with its text - a bare presence assertion on that id cannot fail. The Pro message-length and badge specs take a real grant rather than a display mock: an over-standard-length message carries Pro features the RECIPIENT validates, so without a proof it is composed and sent happily and never arrives.
Three gaps found reviewing the fifteen Pro specs for correctness rather than for whether they pass. The subscribed and expired Pro settings specs asserted only the presence of their own rows, so an app rendering the active-only sections on an expired plan passed both. Each now asserts the other state's rows are absent, which is what distinguishes the screens. Neither pinned-conversation limit spec verified anything was pinned. The pin icon is asserted only on Android, so the iOS half proved nothing beyond "no CTA appeared" - equally true if pinning silently did nothing. Both now assert the conversation order, which works on either platform, and the non-Pro spec additionally asserts the over-limit conversation was NOT pinned: an app that showed the CTA and pinned anyway satisfied a CTA-only assertion. The message-length specs sent to Note to Self, so they only ever proved the sender's composer allows the longer limit. Over-standard-length messages carry Pro features the receiving client validates, so a missing or invalid proof lets the sender compose and send happily while nothing arrives - which Note to Self cannot show. The cases that send now use a second device and assert the recipient rendered the message; the two that cannot send need no recipient and stay on one device. The iOS countdown locator matched an unscoped xpath built by interpolating the expected number, so the absence cases built a selector for the literal "undefined" and passed unconditionally. It now matches the character-limit-text identifier and reads the count from label - the identifier owns name. Needs session-ios#740.
mpretty-cyro
force-pushed
the
feat/desktop-pro-specs
branch
from
August 13, 2026 05:32
eb8ffb2 to
24e5504
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings Desktop up to the same fifteen Session Pro specs that iOS and Android already run, and fixes
three mobile specs that were passing without asserting what they claimed.
Note
Stacked on #126, so only the five commits below belong to this PR.
Needs session-ios#740 — the iOS countdown locator matches an accessibility identifier added there.
The Desktop app-side changes are session-desktop#1979.
Results
Three mobile specs were not asserting what they claimed
Found reviewing the fifteen for correctness rather than for whether they pass.
active-only sections on an expired plan passed both. Each now asserts the other state's rows are
absent — that is what distinguishes the screens.
so the iOS half proved nothing beyond "no CTA appeared" — equally true if pinning silently did
nothing. Both now assert conversation order, and the non-Pro spec asserts the over-limit
conversation was not pinned: an app that showed the CTA and pinned anyway satisfied the old
assertion.
the longer limit. Over-standard-length messages carry Pro features the receiving client
validates, so a missing proof lets the sender send happily while nothing arrives. The sending cases
now use a second device and assert the recipient rendered the message.
Plus the iOS countdown locator, which matched an unscoped xpath built by interpolating the expected
number — so the absence cases built a selector for the literal
undefinedand passedunconditionally. Two specs (
1799and9799 chars) have never actually asserted anything on iOSuntil now.
Desktop harness
@prois derived from a spec declaring a Pro context rather than hand-declared, so the tag cannotdisagree with the setup. Restarts relaunch a window on the same user-data directory, because Session
Desktop asks the Pro backend for status only at startup. The local file server is mapped through —
worth noting it needs the server's Ed25519 key where the mobile clients need its X25519 one; both
are 64 hex characters, so the wrong one fails no format check.
makeAccountPromoves torun/shared/and drops its two mobile type dependencies, so Desktopconsuming it is not a layering violation.
Reviewing
Intermediate commits are not individually buildable — the grant move and the specs that import it
span commits, and splitting them further needed hunk-level staging. The branch tip is what was
verified.