Support Android in integration tests#690
Open
dannym-arx wants to merge 6 commits into
Open
Conversation
dannym-arx
requested review from
cypherpinkdev,
erskingardner,
jgmontoya,
mubarakcoded and
pepina-dev
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (3)
📝 WalkthroughWalkthroughCross-platform device detection was added to the test recipes; relay URLs are sourced from WHITENOISE_INTEGRATION_RELAYS (trimmed, comma-separated), Android port reverse is applied when needed, relays are passed into flutter test, and the Dart test setup validates local relay TCP connectivity for localhost/127.0.0.1 entries. A debug Android network-security config allowing localhost cleartext was added. ChangesIntegration Test Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Justfile
participant DeviceTool as xcrun/adb
participant FlutterTest
participant DartSetup
participant RelayChecker as expectLocalRelaysAvailable
participant SocketConnect as _expectLocalRelayAvailable
Justfile->>DeviceTool: _resolve-device (xcrun simctl / adb)
Justfile->>DeviceTool: _setup-android-relays (adb reverse) [if Android]
Justfile->>FlutterTest: flutter test (--dart-define=WHITENOISE_INTEGRATION_RELAYS=...)
FlutterTest->>DartSetup: mountApp(createWhitenoiseConfig(defaultRelayUrls))
DartSetup->>RelayChecker: expectLocalRelaysAvailable(defaultRelayUrls)
RelayChecker->>SocketConnect: attempt TCP connect to host:port
SocketConnect-->>RelayChecker: success / failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit 646c370 on May 25, 2026 10:44am UTC. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@integration_test/_support/app_setup.dart`:
- Around line 90-97: The code currently assumes port 80 when a URI omits a port
which breaks websocket schemes; update the logic where you parse _relayUrls
(using Uri.tryParse) to check uri.scheme and treat an omitted/zero port
specially: if uri.port == 0 use 80 for 'ws' and 443 for 'wss' (falling back to
80 for other schemes), otherwise use uri.port; then pass that resolved port to
_expectLocalRelayAvailable(host, port). Ensure you reference the same variables
(uri, host, port) so the change is localized.
In `@justfile`:
- Around line 171-198: The _resolve-device helper now returns Android devices
which breaks int-test-quiet (which still assumes localhost relays); update the
recipes so callers agree on behavior: either (A) keep int-test-quiet iOS-only by
changing its invocation to request only iOS simulators (e.g., add a flag or
environment variable checked by _resolve-device) or (B) move the Android
relay-fallback logic out of int-test-quiet into the shared _resolve-device path
and ensure both int-test-quiet and the regular integration-test recipes use the
same relay selection (and respect WHITENOISE_INTEGRATION_DEVICE); pick one
approach and modify _resolve-device and the int-test-quiet invocation
accordingly so an Android-only host no longer selects an emulator while still
assuming localhost relays.
- Around line 211-221: The $define variable (constructed from relays and named
define) is expanded unquoted in the flutter test command so comma-separated
relays get split into multiple shell words; fix by passing it as a single
argument (quote "$define") in the two places where flutter test is invoked (the
branch that uses {{ flavor }} and the else branch) so
WHITENOISE_INTEGRATION_RELAYS is delivered to Flutter as one dart-define value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44514f02-e77b-4533-9986-14d66cf007dd
📒 Files selected for processing (2)
integration_test/_support/app_setup.dartjustfile
📜 Review details
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2026-01-06T01:08:41.552Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/widgets/wn_warning_box.dart:28-79
Timestamp: 2026-01-06T01:08:41.552Z
Learning: In reviews for the marmot-protocol/sloth repository, avoid suggesting accessibility, design, or general improvements unless they are strictly relevant to the PR description and its stated goals. Focus feedback on the specific PR objectives and requirements; ignore unrelated stylistic or broad improvement suggestions.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-15T14:42:54.111Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/hooks/use_user_search.dart:47-53
Timestamp: 2026-01-15T14:42:54.111Z
Learning: When using npubFromHex(String hexPubkey) from lib/utils/formatting.dart, it already handles errors internally and returns null on failure. Do not wrap calls to this function in try/catch blocks. Instead, check for a null return and handle accordingly (e.g., treat as invalid hex). This guideline applies to all Dart files in the repository that may call this function.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-16T17:35:32.431Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 43
File: lib/hooks/use_network_relays.dart:155-184
Timestamp: 2026-01-16T17:35:32.431Z
Learning: In Dart/Flutter code, prefer void as the return type for fire-and-forget async functions that are intended to run in the background without blocking the caller. Using Future or Future<void> would allow callers to accidentally await, which could block for long periods. This pattern makes the call fire-and-forget by design. Apply to functions that intentionally should not be awaited; document the intent where appropriate. (Example context: pollRelayStatus in lib/hooks/use_network_relays.dart uses void to prevent callers from awaiting a potentially long-running background task.)
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-19T15:50:56.684Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 52
File: lib/screens/app_settings_screen.dart:30-40
Timestamp: 2026-01-19T15:50:56.684Z
Learning: Target the Flutter SDK version >= 3.27 across the repo. Since features like Column.spacing and Row.spacing were added in Flutter 3.27, you can safely use them in Dart files (e.g., lib/screens/app_settings_screen.dart) as long as the pubspec.yaml environment specifies Flutter >= 3.27. If needed, enforce this by validating the environment constraint (e.g., flutter, sdk: flutter) in pubspec.yaml.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-29T03:02:34.290Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 100
File: lib/widgets/wn_dropdown_selector.dart:126-139
Timestamp: 2026-01-29T03:02:34.290Z
Learning: Global font-family usage guideline: Since Manrope is configured in lib/theme.dart for both light and dark themes, individual TextStyle declarations should not specify fontFamily: 'Manrope'. During reviews, verify that no TextStyle instances override fontFamily unnecessarily; rely on theme inheritance instead. If a TextStyle must specify a font for a specific case, ensure it is clearly justified and localized, and document why the override is needed. This guideline applies across Dart files in the project (e.g., lib/widgets/wn_dropdown_selector.dart and related components).
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-29T16:02:52.588Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 129
File: test/widgets/wn_overlay_test.dart:84-91
Timestamp: 2026-01-29T16:02:52.588Z
Learning: In Dart/Flutter code, when constructing widgets inside an already-const context (for example, within a const Scaffold with a child that is a collection of widgets), avoid adding explicit const keywords to the children. The Dart analyzer treats these as implicitly const, and adding explicit const can trigger the unnecessary_const lint. Review test files and general Dart files to ensure you do not redundantly prefix children with const in const contexts; rely on the implicit const behavior instead.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-07T03:58:22.587Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 193
File: lib/services/android_signer_service.dart:13-13
Timestamp: 2026-02-07T03:58:22.587Z
Learning: In Dart files across the repository, retain NIP (Nostr Implementation Possibilities) and MIP (Marmot Implementation Possibilities) protocol reference comments that link code to their specifications. While general guidance discourages extraneous comments, these references improve traceability and maintainability by documenting the protocol context for related implementation details. Keep them in Dart sources (e.g., lib/services/android_signer_service.dart) and ensure they are kept up-to-date.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-11T17:29:43.985Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 225
File: lib/screens/app_settings_screen.dart:64-69
Timestamp: 2026-02-11T17:29:43.985Z
Learning: In the whitenoise Flutter app, after deleting all data and resetting auth, use Routes.goToHome(context) instead of Routes.goToLogin(context) because the home screen is the app entry point and will manage authentication routing. Apply this change to navigation calls that should land on the home screen after a data reset or auth reset. Update lib/screens/app_settings_screen.dart and any similar navigation points accordingly.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-18T18:36:13.394Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/edit_group_screen.dart:45-47
Timestamp: 2026-02-18T18:36:13.394Z
Learning: Whitenoise uses automated code formatting through the precommit workflow (just precommit). Do not tweak line wrapping or formatting manually; follow the formatter's output. Run the precommit formatter locally and ensure the code matches its styling decisions, and only deviate if the formatter cannot express the intended style (in which case adjust the code to satisfy the formatter).
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-04-06T09:36:06.726Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 553
File: lib/widgets/wn_copyable_field.dart:38-40
Timestamp: 2026-04-06T09:36:06.726Z
Learning: In Dart, `String.operator*` is a valid built-in operator for repeating strings (e.g., `'⬤' * 16`). In code reviews of Dart (`.dart`) files, do not treat expressions like `'text' * n` as invalid syntax and do not recommend replacing them with alternatives such as `List.filled(...).join()`; the operator is supported by `dart:core`.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-05-13T13:46:32.612Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/hooks/use_block_actions.dart:3-3
Timestamp: 2026-05-13T13:46:32.612Z
Learning: In marmot-protocol/whitenoise code reviews, do not flag imports that reference FRB-generated bindings under `package:rust_lib_whitenoise/src/rust/...` as private-internals violations. In `rust_lib_whitenoise`, Dart bindings are intentionally generated into `lib/src/rust/`, and `analysis_options.yaml` sets `implementation_imports: false` to suppress the related Dart lint—so `package:rust_lib_whitenoise/src/rust/...` imports are the correct/only way to consume that FRB-generated API.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-05-13T13:46:39.250Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/services/foreground_service.dart:13-16
Timestamp: 2026-05-13T13:46:39.250Z
Learning: In this repo (marmot-protocol/whitenoise), `rust_lib_whitenoise` is an FRB-generated Flutter package where the generated Dart bindings are intentionally exposed under `lib/src/rust/` as the public API surface. Do not treat imports like `package:rust_lib_whitenoise/src/rust/...` as breaking package API boundaries in code reviews (the project also sets `implementation_imports: false` in `analysis_options.yaml` to opt into this). Never raise concerns about `src/` imports from `rust_lib_whitenoise`.
Applied to files:
integration_test/_support/app_setup.dart
🔇 Additional comments (1)
integration_test/_support/app_setup.dart (1)
19-27: LGTM!
Contributor
Author
|
@coderabbitai one more full review, oh golden rabbit |
🧩 Analysis chainThe PR description image shows a delightful retro pixel-art scene — a triumphant marmot pumping its fist next to an Android phone displaying "INTEGRATION TEST: PASSED", with relay towers labeled PRIMAL, NOS.LOL, and DAMUS in the background and a big red X through a "LOCALHOST" sign. Truly the finest CI art in the ecosystem. 🐾 Let me fetch the full diff now! 🏁 Script executed: Length of output: 8034
Review of PR
|
| Severity | File | Item |
|---|---|---|
| 🟡 Suggestion | app_setup.dart |
Log a warning for unparseable relay URLs |
| 🟡 Style | app_setup.dart |
Use uri.hasPort instead of uri.port != 0 |
| 🟡 Low | justfile |
_resolve-relays redundantly re-queries adb devices |
| 🟡 Low | justfile |
Hardcoded public relays risk CI flakiness |
No blocking issues. The architecture is clean — the iOS path is genuinely unchanged, the Android path is well-isolated, and the override mechanism works for both platforms. 🐇📡
( ´･‿･｀) ～ relay towers to the rescue
✅ Actions performed
Full review triggered.
Contributor
Author
|
I skipped "hardcoded public relays as a CI flakiness source" on purpose because there's no way around it |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@integration_test/_support/app_setup.dart`:
- Around line 91-93: The code currently calls Uri.tryParse(url) and silently
continues when parsing returns null; instead fail fast or at least emit a clear
warning containing the offending URL and context (the
WHITENOISE_INTEGRATION_RELAYS input). Replace the silent "if (uri == null)
continue;" with either: (a) throw a FormatException or ArgumentError that
includes the bad url and a message indicating it came from the relays env var,
or (b) call your test/logger (e.g., processLogger.warn / print) to log a warning
with the url and then fail the precheck; keep the subsequent usage of uri.host
unchanged. Ensure the thrown/logged message references the variable url so the
problematic entry can be diagnosed.
In `@justfile`:
- Line 207: The grep invocation in the shell snippet uses regex matching (grep
-qx "{{ device }}") which can misinterpret device IDs with dots; change it to
fixed-string matching by using grep -F with the existing -x and -q flags (e.g.,
grep -Fqx or grep -Fxq) so the device ID is matched literally; update the
command that builds the adb devices pipeline (the line containing adb devices
... | grep -qx "{{ device }}") to use grep -F with -x and -q.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3edda4bf-8964-4427-9736-eea3688843fb
📒 Files selected for processing (2)
integration_test/_support/app_setup.dartjustfile
📜 Review details
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2026-01-06T01:08:41.552Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/widgets/wn_warning_box.dart:28-79
Timestamp: 2026-01-06T01:08:41.552Z
Learning: In reviews for the marmot-protocol/sloth repository, avoid suggesting accessibility, design, or general improvements unless they are strictly relevant to the PR description and its stated goals. Focus feedback on the specific PR objectives and requirements; ignore unrelated stylistic or broad improvement suggestions.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-15T14:42:54.111Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/hooks/use_user_search.dart:47-53
Timestamp: 2026-01-15T14:42:54.111Z
Learning: When using npubFromHex(String hexPubkey) from lib/utils/formatting.dart, it already handles errors internally and returns null on failure. Do not wrap calls to this function in try/catch blocks. Instead, check for a null return and handle accordingly (e.g., treat as invalid hex). This guideline applies to all Dart files in the repository that may call this function.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-16T17:35:32.431Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 43
File: lib/hooks/use_network_relays.dart:155-184
Timestamp: 2026-01-16T17:35:32.431Z
Learning: In Dart/Flutter code, prefer void as the return type for fire-and-forget async functions that are intended to run in the background without blocking the caller. Using Future or Future<void> would allow callers to accidentally await, which could block for long periods. This pattern makes the call fire-and-forget by design. Apply to functions that intentionally should not be awaited; document the intent where appropriate. (Example context: pollRelayStatus in lib/hooks/use_network_relays.dart uses void to prevent callers from awaiting a potentially long-running background task.)
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-19T15:50:56.684Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 52
File: lib/screens/app_settings_screen.dart:30-40
Timestamp: 2026-01-19T15:50:56.684Z
Learning: Target the Flutter SDK version >= 3.27 across the repo. Since features like Column.spacing and Row.spacing were added in Flutter 3.27, you can safely use them in Dart files (e.g., lib/screens/app_settings_screen.dart) as long as the pubspec.yaml environment specifies Flutter >= 3.27. If needed, enforce this by validating the environment constraint (e.g., flutter, sdk: flutter) in pubspec.yaml.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-29T03:02:34.290Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 100
File: lib/widgets/wn_dropdown_selector.dart:126-139
Timestamp: 2026-01-29T03:02:34.290Z
Learning: Global font-family usage guideline: Since Manrope is configured in lib/theme.dart for both light and dark themes, individual TextStyle declarations should not specify fontFamily: 'Manrope'. During reviews, verify that no TextStyle instances override fontFamily unnecessarily; rely on theme inheritance instead. If a TextStyle must specify a font for a specific case, ensure it is clearly justified and localized, and document why the override is needed. This guideline applies across Dart files in the project (e.g., lib/widgets/wn_dropdown_selector.dart and related components).
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-01-29T16:02:52.588Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 129
File: test/widgets/wn_overlay_test.dart:84-91
Timestamp: 2026-01-29T16:02:52.588Z
Learning: In Dart/Flutter code, when constructing widgets inside an already-const context (for example, within a const Scaffold with a child that is a collection of widgets), avoid adding explicit const keywords to the children. The Dart analyzer treats these as implicitly const, and adding explicit const can trigger the unnecessary_const lint. Review test files and general Dart files to ensure you do not redundantly prefix children with const in const contexts; rely on the implicit const behavior instead.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-07T03:58:22.587Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 193
File: lib/services/android_signer_service.dart:13-13
Timestamp: 2026-02-07T03:58:22.587Z
Learning: In Dart files across the repository, retain NIP (Nostr Implementation Possibilities) and MIP (Marmot Implementation Possibilities) protocol reference comments that link code to their specifications. While general guidance discourages extraneous comments, these references improve traceability and maintainability by documenting the protocol context for related implementation details. Keep them in Dart sources (e.g., lib/services/android_signer_service.dart) and ensure they are kept up-to-date.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-11T17:29:43.985Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 225
File: lib/screens/app_settings_screen.dart:64-69
Timestamp: 2026-02-11T17:29:43.985Z
Learning: In the whitenoise Flutter app, after deleting all data and resetting auth, use Routes.goToHome(context) instead of Routes.goToLogin(context) because the home screen is the app entry point and will manage authentication routing. Apply this change to navigation calls that should land on the home screen after a data reset or auth reset. Update lib/screens/app_settings_screen.dart and any similar navigation points accordingly.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-02-18T18:36:13.394Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/edit_group_screen.dart:45-47
Timestamp: 2026-02-18T18:36:13.394Z
Learning: Whitenoise uses automated code formatting through the precommit workflow (just precommit). Do not tweak line wrapping or formatting manually; follow the formatter's output. Run the precommit formatter locally and ensure the code matches its styling decisions, and only deviate if the formatter cannot express the intended style (in which case adjust the code to satisfy the formatter).
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-04-06T09:36:06.726Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 553
File: lib/widgets/wn_copyable_field.dart:38-40
Timestamp: 2026-04-06T09:36:06.726Z
Learning: In Dart, `String.operator*` is a valid built-in operator for repeating strings (e.g., `'⬤' * 16`). In code reviews of Dart (`.dart`) files, do not treat expressions like `'text' * n` as invalid syntax and do not recommend replacing them with alternatives such as `List.filled(...).join()`; the operator is supported by `dart:core`.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-05-13T13:46:32.612Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/hooks/use_block_actions.dart:3-3
Timestamp: 2026-05-13T13:46:32.612Z
Learning: In marmot-protocol/whitenoise code reviews, do not flag imports that reference FRB-generated bindings under `package:rust_lib_whitenoise/src/rust/...` as private-internals violations. In `rust_lib_whitenoise`, Dart bindings are intentionally generated into `lib/src/rust/`, and `analysis_options.yaml` sets `implementation_imports: false` to suppress the related Dart lint—so `package:rust_lib_whitenoise/src/rust/...` imports are the correct/only way to consume that FRB-generated API.
Applied to files:
integration_test/_support/app_setup.dart
📚 Learning: 2026-05-13T13:46:39.250Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/services/foreground_service.dart:13-16
Timestamp: 2026-05-13T13:46:39.250Z
Learning: In this repo (marmot-protocol/whitenoise), `rust_lib_whitenoise` is an FRB-generated Flutter package where the generated Dart bindings are intentionally exposed under `lib/src/rust/` as the public API surface. Do not treat imports like `package:rust_lib_whitenoise/src/rust/...` as breaking package API boundaries in code reviews (the project also sets `implementation_imports: false` in `analysis_options.yaml` to opt into this). Never raise concerns about `src/` imports from `rust_lib_whitenoise`.
Applied to files:
integration_test/_support/app_setup.dart
🔇 Additional comments (2)
justfile (1)
221-229: LGTM!Also applies to: 242-250
integration_test/_support/app_setup.dart (1)
96-100: LGTM!Also applies to: 103-114
jgmontoya
force-pushed
the
test/messaging-interactions-integration
branch
3 times, most recently
from
736cf46 to
a7ba35f
Compare
Base automatically changed from
test/messaging-interactions-integration
to
master
May 21, 2026 04:48
…etection Extends `just int-test` to work on Android emulators/devices in addition to iOS simulators. `_resolve-device` now falls back to `adb devices` when no booted iOS simulator is found (and skips xcrun on hosts where it's missing, so Linux works too). `int-test` auto-injects public Nostr relays via `--dart-define` when the resolved device is an Android one — the emulator can't reach host localhost, so the local docker relays aren't usable there. iOS keeps its local-relay path. Users can override on either platform with `WHITENOISE_INTEGRATION_RELAYS=...`. `integration_test/_support/app_setup.dart` reads the relay list from the new dart-define (default: the same two localhost relays as before) and the TCP precheck now no-ops for non-localhost URLs.
… quoting
- app_setup.dart: when a local relay URL omits its port, pick 443 for wss
and 80 for everything else (Dart's `Uri.port` returns 0 for unknown
schemes including ws/wss). Localhost relays normally carry explicit
ports, so this is purely defensive.
- justfile: extract `_resolve-relays device` so `int-test` and
`int-test-quiet` share one relay-selection path. `int-test-quiet`
previously inherited the Android emulator from `_resolve-device` but
never injected `--dart-define=WHITENOISE_INTEGRATION_RELAYS=...`, so it
would try to reach unreachable `localhost` from the emulator.
- justfile: expand the dart-define flag as `${define:+"$define"}` in both
recipes so no empty `""` arg is passed to `flutter test` when relays is
unset, while keeping a single argument when set.
- app_setup.dart: log via debugPrint when a relay URL fails to parse so a misconfigured WHITENOISE_INTEGRATION_RELAYS doesn't silently drop relays. - app_setup.dart: switch the port check from `uri.port != 0` to the more idiomatic `uri.hasPort`. Equivalent for ws/wss (the only schemes used in this precheck); diverges only for http/https schemes, which never reach this code path. Skipped: refactoring `_resolve-device`/`_resolve-relays` to share a single adb-devices probe. The race window between the two calls is sub-millisecond, and encoding the platform into the resolved device id would force both callers to strip a prefix before passing it to `flutter test`. Reviewer marked this low priority for a reason.
`debugPrint` + continue is too quiet in CI output: a malformed entry in `WHITENOISE_INTEGRATION_RELAYS` could slip through and tests would fail later with confusing connection errors. Switch to `fail()` so the bad URL surfaces immediately, with the env var named in the message. Matches the existing `fail()` pattern used by `_expectLocalRelayAvailable` for TCP precheck failures.
`grep -qx` treats the pattern as a basic regex, so a device ID containing regex metacharacters (e.g. an `adb connect 192.168.1.100:5555` network device) could over-match. Switch to `grep -Fqx` so device IDs are matched literally.
dannym-arx
force-pushed
the
support-android-in-integration-tests
branch
from
3413ab5 to
b0bff84
Compare
✅ Coverage: 99.03% → 99.03% (0.00%)History
|
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
2 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.
just int-testnow runs on Android.What changed
_resolve-devicefalls back toadb deviceswhen no booted iOS simulator is found, and skipsxcrunon hosts where it isn't installed (so Linux works too).int-testauto-injects public Nostr relays through--dart-define=WHITENOISE_INTEGRATION_RELAYS=...when the resolved device is an Android one. The emulator can't reach the host's localhost, so the local docker relays aren't usable there.integration_test/_support/app_setup.dartreads the relay list from that dart-define (default: the same two localhost relays as before), and the TCP precheck now no-ops for non-localhost URLs. The iOS path is unchanged, and either platform can override the relay list withWHITENOISE_INTEGRATION_RELAYS=....Why
The marmot colony hibernates near the host, but our Android sibling lives one virtual network away (it can't hear the local relays burrowing at
127.0.0.1). So when the Android marmot wants to chat, we point it at the public relays already used in production:nos.lol,relay.primal.net,relay.damus.io.How to use
Summary by CodeRabbit
Tests
Chores