Add mobile settings - #92
Merged
Merged
Conversation
…est override versions - gofmt server/config.go struct alignment - pass cancellable context through createSingleCMTInstance / createMobileCMTInstances so sibling goroutines stop provisioning when any platform fails - cap explicit CMTServerVersions override by keeping the newest (tail) entries instead of the oldest (head), since the matrix is normally sorted ascending Tests: go vet ./..., go build ./..., go test -race ./... pass. Downstream workflow contract checks (e2e-detox-pr.yml inputs and compatibility-matrix-testing.yml CMT_MATRIX schema) remain to be verified against the actual workflow files before merge.
mm-cloud-bot
added
the
release-note-none
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughMobile E2E orchestration now provisions five platform-specific instances and dispatches explicit URLs. CMT version selection uses semver-aware ESR capping, while mobile CMT provisioning and matrix output represent each platform separately. ChangesMobile E2E and CMT changes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant E2ERequestHandler
participant createCloudInstallation
participant MobileInstances
participant dispatchMobileE2EWorkflow
E2ERequestHandler->>createCloudInstallation: create mobile installation
E2ERequestHandler->>MobileInstances: provision five canonical platforms
MobileInstances->>dispatchMobileE2EWorkflow: provide five platform URLs
dispatchMobileE2EWorkflow->>dispatchMobileE2EWorkflow: validate and map workflow inputs
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/e2e_tests_test.go (1)
649-650: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert against
mobileE2EPlatformsrather than restating the list.The inline predicate hard-codes the same five identifiers the table already provides, so this subtest passes no matter what production code does and will silently drift if the canonical list changes.
♻️ Proposed refactor
- isMobile := tt.platform == "android-site-1" || tt.platform == "android-site-2" || - tt.platform == "ios-site-1" || tt.platform == "ios-site-2" || tt.platform == "site-3" + isMobile := slices.Contains(mobileE2EPlatforms, tt.platform)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/e2e_tests_test.go` around lines 649 - 650, Replace the inline platform comparisons in the subtest’s isMobile calculation with an assertion against the canonical mobileE2EPlatforms collection. Reuse mobileE2EPlatforms so the test validates membership from the shared list and stays synchronized when it changes.server/e2e_tests.go (1)
599-611: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMobile site URLs are plumbed positionally instead of by
Platform. Every mobile dispatch and matrix path assumes instances arrive in exactlymobileE2EPlatformsorder and that the two package-level slices stay the same length; nothing asserts either invariant, so a future reordering or list change swaps Android/iOS URLs silently. A single platform-keyed helper that resolves the five URLs from[]*E2EInstancewould fix all four sites.
server/e2e_tests.go#L599-L611: build aPlatform → URLmap and pairmobileE2EPlatforms[i]withmobileE2EWorkflowInputKeys[i], erroring on a missing platform.server/e2e_tests.go#L1379-L1401: replace the five positional URL parameters with the ordered instance slice (or a struct) and derive the input keys frommobileE2EWorkflowInputKeys.server/push_events.go#L298-L320: stop indexinginstances[0]..instances[4]; resolve URLs through the same platform-keyed helper so the guard and the dispatch cannot diverge.server/workflow_run.go#L579-L587: within each version block, pick each URL by the instance'sPlatformrather than byoffset+N.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/e2e_tests.go` around lines 599 - 611, Replace positional mobile instance URL handling with one platform-keyed helper that maps each instance’s Platform to its URL, validates the mobileE2EPlatforms and mobileE2EWorkflowInputKeys pairing, and errors when a required platform is missing. In server/e2e_tests.go lines 599-611, use the helper for workflow inputs; in server/e2e_tests.go lines 1379-1401, pass the ordered instance slice or equivalent and derive keys through the helper; in server/push_events.go lines 298-320, use the helper for both validation and dispatch instead of instances[0]..instances[4]; in server/workflow_run.go lines 579-587, select URLs by each instance’s Platform rather than offset-based indexing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/e2e_tests_test.go`:
- Around line 649-650: Replace the inline platform comparisons in the subtest’s
isMobile calculation with an assertion against the canonical mobileE2EPlatforms
collection. Reuse mobileE2EPlatforms so the test validates membership from the
shared list and stays synchronized when it changes.
In `@server/e2e_tests.go`:
- Around line 599-611: Replace positional mobile instance URL handling with one
platform-keyed helper that maps each instance’s Platform to its URL, validates
the mobileE2EPlatforms and mobileE2EWorkflowInputKeys pairing, and errors when a
required platform is missing. In server/e2e_tests.go lines 599-611, use the
helper for workflow inputs; in server/e2e_tests.go lines 1379-1401, pass the
ordered instance slice or equivalent and derive keys through the helper; in
server/push_events.go lines 298-320, use the helper for both validation and
dispatch instead of instances[0]..instances[4]; in server/workflow_run.go lines
579-587, select URLs by each instance’s Platform rather than offset-based
indexing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc178ee3-6cce-4ca4-a838-9a8d0373ecf4
📒 Files selected for processing (8)
config/config-matterwick.default.jsonserver/config.goserver/e2e_dryrun_test.goserver/e2e_tests.goserver/e2e_tests_test.goserver/push_events.goserver/workflow_run.goserver/workflow_run_test.go
- Add buildMobileURLInputs helper that validates canonical platforms and pairs them with workflow input keys so callers cannot drift from slice order. - Use the helper in triggerMobileE2EWorkflow, dispatchMobileE2EWorkflow, triggerMobileE2EWorkflowForPushEvent, and buildMobileCMTMatrixJSON. - Replace dispatchMobileE2EWorkflow's five positional URL params with the ordered instance slice. - In e2e_tests_test.go, assert mobile membership via slices.Contains against mobileE2EPlatforms instead of restating the list. Tests: gofmt, go vet, go build, go test -race pass.
saturninoabril
approved these changes
Jul 25, 2026
saturninoabril
left a comment
saturninoabril
left a comment
There was a problem hiding this comment.
Thanks @yasserfaraazkhan, lgtm.
This was referenced Jul 31, 2026
Merged
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
3 participants
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.
Summary: