Add mobile tests - #102
Merged
Merged
Conversation
Adds a React Native app in mobile/ (Expo SDK 57, React 19, RN 0.86,
expo-router, TypeScript) and the Rails API it talks to.
Rails side:
- Api::BaseController inherits ActionController::API, so the phone is
not subject to CSRF, cookies or the allow_browser version check, and
renders 404/400/422 as predictable JSON.
- Api::V1::TasksController exposes CRUD at /api/v1/tasks. Task is a
placeholder resource wired end to end, meant to be replaced.
- rack-cors for the Expo web target. Native builds ignore CORS.
- Development Host Authorization now accepts .local hostnames, which
were 403ing requests from phones resolving the host over mDNS.
Mobile side:
- src/lib/api.ts derives the API host from the Metro bundler address
instead of hardcoding localhost, which resolves differently on the
iOS simulator, the Android emulator (10.0.2.2) and real devices.
EXPO_PUBLIC_API_URL overrides it and is required for release builds.
- Tasks screen lists, creates, toggles and deletes through the API,
updating optimistically and rolling back on failure.
Housekeeping: rubocop, brakeman and the Docker build context skip
mobile/, whose node_modules ship Ruby CocoaPods scripts that would
otherwise be linted and scanned as our own. WSL Zone.Identifier files
are now gitignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit a6fb86f)
The SDK 57 rescaffold reset several things that had been customised on
the earlier add-expo-mobile branch. Carried them across:
- app.json: name "Along With You", slug along-with-you, and the
alongwithyou:// URL scheme. The scaffold defaults meant the app
installed on a phone as "mobile" and claimed the mobile:// scheme,
which matters as soon as OAuth callbacks or push notifications
need a deep link.
- package.json: name alongwithyou-mobile, plus the typecheck script.
- engines.node, set to >=20.19.4 rather than the previous >=20.19.0,
because React Native 0.86 will not build below that.
- eslint, installed at the versions matching SDK 57 rather than the
SDK 54 ones, and mobile/README.md rewritten for the src/ layout and
the API client, which the old copy predates.
Two react-hooks/set-state-in-effect errors surfaced once linting ran.
Loading tasks no longer sets state before its first await, which was a
real cascading render. The remaining report on the mount effect, and one
in Expo's own use-color-scheme.web.ts, are suppressed inline: both want
Suspense or a data library, which this app does not have yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 27d15c0)
Two error paths escaped the JSON contract Api::BaseController sets up, and both are ones a phone hits in practice: - A truncated or malformed request body raised ActionDispatch::Http::Parameters::ParseError, which is handled by middleware and rendered as an HTML error page. The client's JSON.parse then threw a SyntaxError instead of surfacing the real problem. - Any unmatched path under /api fell through to the HTML 404 page. Also drop the new/edit routes, which only make sense for HTML forms and would otherwise shadow the JSON 404 with a missing action error. The newest-first index test could not fail before this: fixtures share a single created_at, so the order was up to SQLite and the assertion held either way. Fixtures now carry explicit timestamps and the test asserts the ids it expects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Requests had no timeout. A network that drops packets rather than refusing the connection - a phone off the wifi, a laptop asleep behind a port forward - left fetch pending forever and the Tasks spinner with it. - JSON.parse ran unguarded on the response body, so an HTML page from a proxy, a tunnel or a Rails error outside /api threw a bare SyntaxError instead of an ApiError, and the UI showed "Something went wrong." - Rolling back a failed delete restored a snapshot of the whole list, undoing anything that changed while the request was in flight. Two deletes in a row could resurrect the first row. Restore by position. - npm run reset-project deletes src/ and scripts/. That is fine in a fresh Expo template; here it deletes the app. - The web tab bar introduced the app as "Expo Starter". types/expo.d.ts holds the reference Expo generates into the gitignored expo-env.d.ts, without which `npm run typecheck` fails on a fresh clone (cannot find '@/global.css', '../animated-icon.module.css'). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
14k lines of TypeScript arrived with nothing checking them: CI runs rubocop, brakeman and the Ruby tests only, and dependabot watched bundler and github-actions but not mobile/package.json, so 822 npm packages had no update or advisory coverage at all. The npm audit gate is set to high: Expo's build toolchain currently pulls in uuid < 11.1.1 through @expo/config-plugins, which nothing here can resolve, and 10 packages depend on it. Also correct the test commands in the README (`npx tsc --noEmit` is `npm run typecheck`, and `bin/ci` is Ruby-only), and record the follow-ups this review turned up but did not change: rate limiting, app store identifiers, the leftover Expo template screens, and the licensing question `mobile/LICENSE` raises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A 12.8 MB core dump from React Native DevTools (mobile/core.9043) came with this work and is dropped from the branch. mobile/.gitignore already lists core.*; the root did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tasks.completed is NOT NULL with nothing above the database enforcing it,
and the column is permitted in task_params. PATCH with {"completed":null}
reached SQLite and raised ActiveRecord::NotNullViolation: a 500 with an
HTML body, from the controller whose whole job is predictable JSON.
Values that cast to a boolean still pass, so "1", "0", "true" and "false"
behave as before. Only nil - an explicit null, or "" - is rejected, and it
comes back as a field error the client can render.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jest-expo plus React Native Testing Library, wired into the mobile CI job. Nineteen tests across the two pieces most worth covering first: - src/lib/api.test.ts drives the client against a mocked fetch: how the API URL is resolved for a release build, the Android emulator, a LAN device and a bare localhost, and what each failure becomes - a 422 with field errors, an HTML body, an unreachable host, a server that never answers. - src/__tests__/tasks-screen.test.tsx drives the screen against a mocked client: listing, adding, toggling, retrying after a failed load, and keeping a row that failed to delete. Two things this setup has to work around, both documented where they bite: - Expo Router turns every file under src/app/ into a route, and its context regex only skips +api, +html and +middleware. A test beside a screen would ship as a route, so screen tests live in src/__tests__/. - TypeScript 6 does not pull @types/jest in automatically here, so each test file references it directly rather than putting test-only types in tsconfig.json. Writing the tests turned up one thing worth knowing about the client: in development it always resolves a URL, falling back to localhost when Metro never reported a host, so the "No API URL configured" path only exists in a release build. Both behaviours are now pinned by tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
No description provided.