QA: allow a launch extra to name the file server - #2183
Open
mpretty-cyro wants to merge 1 commit into
Open
Conversation
The debug menu can already select an alternative file server, but only from a hardcoded list of remote test servers -- there was no way to name an arbitrary one, so a locally hosted file server was unreachable on this platform. That blocked more than convenience. iOS takes the same pair as launch args and Desktop as environment variables, so with a local file server configured Android uploaded to production while the others read from the local one: a cross-platform attachment test involving Android could not work at all, and none has ever existed. The upload and the download were looking at different servers. Writes the existing `alternativeFileServer` preference rather than adding a parallel one, so the app keeps a single notion of "not the production file server", and `AttachmentUploadJob` reads it live at upload time. ED25519, matching what `FileServer` stores and what Desktop is given -- NOT the X25519 form iOS takes, which `FileServer.x25519PubKeyHex` derives itself. Both-or-neither and present-but-empty-clears, as the Pro backend pair does, and registered in the recognised-extras list so a typo is still reported rather than silently doing nothing. Verified on device: "Setting file server override to http://192.168.139.2:8000", then "Attachment uploaded successfully." from AttachmentUploadJob.
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.
Adds
sessionFileServerUrl/sessionFileServerPubkeylaunch extras, so a QA run can point the app at a file server it names.Why
The debug menu can already select an alternative file server, but only from a hardcoded list of remote test servers (
TEST_FILE_SERVERS). There was no way to name an arbitrary one, so a locally-hosted file server was unreachable on this platform.That blocked more than convenience. The other two clients can already be told: iOS takes the same pair as
customFileServerUrl/customFileServerPubkeylaunch args, Desktop takesTEST_FILE_SERVER_URL/TEST_FILE_SERVER_ED_PK. Android could not — so with a local file server configured, Android uploaded to production while the other client read from the local one, and the download simply never resolved.The consequence in the test suite is that no Android attachment has ever been verified reaching another platform, because it could not be. There are no cross-platform media specs at all today, and this is why.
What it does
alternativeFileServerpreference rather than adding a parallel one, so the app keeps a single notion of "not the production file server".AttachmentUploadJobandAvatarUploadManagerread it live at upload time.FileServerstores and what Desktop is given — not the X25519 form iOS takes.FileServer.x25519PubKeyHexderives that itself, so passing the X25519 key here would be wrong twice over.sessionProBackendUrlpair it is modelled on.warnOnUnrecognisedExtrasrather than silently doing nothing.ALLOW_QA_LAUNCH_CONFIGis set, like the rest ofQaLaunchConfig.Verification
Not inferred from the build — confirmed on device. With the extras passed:
and the receiving client then renders the attachment. Before this, the same test waited out a 60s download timeout on every attempt.
Note for reviewers
A wrong key here fails inside the onion request rather than at configuration time, so a mismatch presents as a download that never resolves rather than as an error. That is the reasoning behind rejecting a half-supplied pair instead of half-applying it, and behind the explicit Ed25519 length/charset check.