feat: added "apps foreground" command, improved tested coverage for iOS simulators.#165
feat: added "apps foreground" command, improved tested coverage for iOS simulators.#165
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds foreground-app retrieval across CLI, command layer, and device implementations; extends tests and tooling to run coverage-instrumented e2e simulator tests and documents new app-management commands. (44 words) Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 78: In the README line that reads "`mobilecli` will automatically install
an agent on the device that is required for functions such as tapping on
elements, pressing buttons buttons and streaming screen capture.", remove the
duplicated word so it reads "pressing buttons" (i.e., change "pressing buttons
buttons" to "pressing buttons").
In `@test/server.ts`:
- Around line 25-29: The createCoverageDirectory function currently always makes
a random dir; change it to first check process.env.GOCOVERDIR and return that
path (creating it if needed) and only fall back to generating the random dir
(using path.join(__dirname, "cover-"+randomUUID())) when GOCOVERDIR is not set;
ensure you call mkdirSync on the chosen path (with recursive if required) before
returning. Update the identical pattern in test/simulator.ts (the function
at/around the coverage-dir creation) so both files respect GOCOVERDIR and create
the directory if missing.
In `@test/simctl.ts`:
- Around line 114-120: The cleanupSimulators function mutates createdSimulators
during forEach because deleteSimulator removes entries, which can skip items;
fix by iterating over a stable snapshot (e.g., use a shallow copy of
createdSimulators or iterate from the end) so each simulatorId is guaranteed to
be processed, calling shutdownSimulator(simulatorId) and
deleteSimulator(simulatorId) for every id, then clear createdSimulators.length =
0 as before; reference functions/variables: cleanupSimulators,
createdSimulators, shutdownSimulator, deleteSimulator.
🧹 Nitpick comments (2)
test/simulator.ts (2)
24-24: Consider making the iOS versions configurable.Hard-coding only
'26'risks skipping tests on most dev/CI hosts. Consider deriving from available runtimes or using an env var to keep coverage broad without editing the file.
78-148: Reduce flakiness by replacing fixed sleeps with polling.Multiple tests rely on fixed delays (e.g., 10s/3s) which can be flaky across CI loads and device states. Consider polling foreground-app/UI state until ready or timeout.
| const createCoverageDirectory = (): string => { | ||
| const dir = path.join(__dirname, "cover-" + randomUUID()); | ||
| mkdirSync(dir); | ||
| return dir; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the test/server.ts file
fd "server.ts" test/Repository: mobile-next/mobilecli
Length of output: 79
🏁 Script executed:
# Check if test/server.ts exists and examine the relevant lines
if [ -f "test/server.ts" ]; then
wc -l "test/server.ts"
echo "--- Lines 20-35 ---"
sed -n '20,35p' "test/server.ts"
fiRepository: mobile-next/mobilecli
Length of output: 506
🏁 Script executed:
# Search for startTestServer function to see if it sets GOCOVERDIR
rg "startTestServer" -A 10 -B 2Repository: mobile-next/mobilecli
Length of output: 1201
🏁 Script executed:
# Look for GOCOVERDIR usage in the codebase
rg "GOCOVERDIR" -nRepository: mobile-next/mobilecli
Length of output: 163
🏁 Script executed:
# Check for Makefile and coverage-related targets
fd "Makefile" . && echo "Found Makefile"Repository: mobile-next/mobilecli
Length of output: 90
🏁 Script executed:
# Examine the Makefile for coverage-related targets
cat MakefileRepository: mobile-next/mobilecli
Length of output: 763
🏁 Script executed:
# Check test/simulator.ts to see the similar GOCOVERDIR override pattern
sed -n '160,180p' "test/simulator.ts"Repository: mobile-next/mobilecli
Length of output: 633
🏁 Script executed:
# Search for any coverage aggregation scripts or setup
rg "coverage" -n --type-list | grep -E "Makefile|\.sh|package.json"
fd "\.(sh|json)$" . | xargs grep -l "coverage" 2>/dev/null | head -20Repository: mobile-next/mobilecli
Length of output: 454
🏁 Script executed:
# Check package.json to see how test-simulator is defined
fd "package.json" test/
cat test/package.json | grep -A 5 "test-simulator"Repository: mobile-next/mobilecli
Length of output: 224
🏁 Script executed:
# Search for any scripts that might set GOCOVERDIR before running tests
rg "GOCOVERDIR" -B 5 -A 5Repository: mobile-next/mobilecli
Length of output: 1032
🏁 Script executed:
# Check if there's any setup that pre-sets GOCOVERDIR in the e2e flow
grep -r "GOCOVERDIR" . --include="*.sh" --include="*.json" --include="*.ts" --include="*.js" --include="Makefile" 2>/dev/null | head -20Repository: mobile-next/mobilecli
Length of output: 159
Respect a pre-set GOCOVERDIR environment variable for proper coverage aggregation.
The createCoverageDirectory function always generates a new random directory, which prevents the e2e coverage flow from working correctly. The Makefile's test-e2e target expects coverage data in test/coverage and uses go tool covdata textfmt -i=test/coverage to aggregate it, but coverage is instead written to random directories (e.g., test/cover-<UUID>). Check for a pre-set GOCOVERDIR environment variable first and use it if available; only generate a random directory as a fallback.
Note: This same pattern exists in test/simulator.ts at line 170 and should be updated consistently.
Proposed fix
-const createCoverageDirectory = (): string => {
- const dir = path.join(__dirname, "cover-" + randomUUID());
- mkdirSync(dir);
- return dir;
-}
+const createCoverageDirectory = (): string => {
+ const envDir = process.env.GOCOVERDIR;
+ if (envDir) {
+ mkdirSync(envDir, {recursive: true});
+ return envDir;
+ }
+ const dir = path.join(__dirname, "cover-" + randomUUID());
+ mkdirSync(dir, {recursive: true});
+ return dir;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createCoverageDirectory = (): string => { | |
| const dir = path.join(__dirname, "cover-" + randomUUID()); | |
| mkdirSync(dir); | |
| return dir; | |
| } | |
| const createCoverageDirectory = (): string => { | |
| const envDir = process.env.GOCOVERDIR; | |
| if (envDir) { | |
| mkdirSync(envDir, {recursive: true}); | |
| return envDir; | |
| } | |
| const dir = path.join(__dirname, "cover-" + randomUUID()); | |
| mkdirSync(dir, {recursive: true}); | |
| return dir; | |
| } |
🤖 Prompt for AI Agents
In `@test/server.ts` around lines 25 - 29, The createCoverageDirectory function
currently always makes a random dir; change it to first check
process.env.GOCOVERDIR and return that path (creating it if needed) and only
fall back to generating the random dir (using path.join(__dirname,
"cover-"+randomUUID())) when GOCOVERDIR is not set; ensure you call mkdirSync on
the chosen path (with recursive if required) before returning. Update the
identical pattern in test/simulator.ts (the function at/around the coverage-dir
creation) so both files respect GOCOVERDIR and create the directory if missing.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
apps foregroundcommand and device APIs to report the currently foreground app (package, name, version).Documentation
Tests
Chores