[Integration] Integration test infrastructure and cross-system bug fixes#286
Conversation
- Fix missing closing brace in RepositoryModule.provideConfigRepository that caused checkstyle's ANTLR parser to crash - Exclude @category(IntegrationTest.class) tests from the unit test run in build.gradle.kts so e2e tests (which require a live server) do not fail CI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to avoid leaking information about ID validity
…ices from 400/403 to 404 to avoid leaking information
…avoid leaking information
…avoid leaking information
…ration test cleanup
…rt network services
…nd attachment IDs
|
Hey @nemoshu, I'm still polishing this up (getting it passing CI/CD and resolving merge conflicts) - but most of the bones are in place now in terms of integration bug fixes if you want to start taking a look |
|
I've added more test cases:
|
…inary contamination
windows/ManuscriptaTeacherApp/Main/Controllers/DistributionController.cs
Outdated
Show resolved
Hide resolved
windows/ManuscriptaTeacherApp/Main/Controllers/FeedbackController.cs
Outdated
Show resolved
Hide resolved
windows/ManuscriptaTeacherApp/Main/Controllers/ConfigController.cs
Outdated
Show resolved
Hide resolved
Per PR #286 discussion: it is unreasonable to return 404 for malformed GUIDs (incorrect format) — the original 400 semantics should be retained. 404 is kept for the config endpoint when the device is valid but unknown or non-Android. Controllers (4 files): - AttachmentController, ConfigController, DistributionController, FeedbackController: Guid.TryParse failure now returns 400 BadRequest with descriptive messages (e.g. 'Invalid device ID format') instead of 404 NotFound. - ConfigController: non-Android device catch block remains 404. Windows unit tests (4 files): - Updated assertions from NotFoundObjectResult to BadRequestObjectResult and renamed test methods accordingly. Android integration tests (4 files): - Renamed existing malformed-GUID tests to *_malformedId_returns400 / *_malformedDeviceId_returns400 and changed assertions to expect 400. - Added new *_nonexistentId_returns404 / *_nonexistentDevice_returns404 tests using a valid but nonexistent UUID (ffffffff-...) to properly cover 404 behaviour. Documentation (2 files): - API Contract: added Path Parameter Validation rule; added 400 Bad Request error responses to attachments, config, distribution, feedback. - Integration Test Contract: added malformed-ID 400 rows to all endpoint tables; changed config non-client device from 403 Forbidden to 404.
Summary
This PR resolves all integration test failures between the Android client and Windows server, bringing both test suites to green. Changes span the Android app, Android integration tests, Windows server controllers and services, Windows unit tests, and the Integration Test Contract documentation.
Changes
1. API URL prefix fix
Files:
ApiService.java,ApiServiceTest.javaAll 7 Retrofit endpoint annotations were missing the
/api/v1/prefix that the server expects (per API Contract §4). Added the prefix to every endpoint and updated the corresponding unit test URL assertions.2. Test entity IDs -> valid UUIDs
Files:
IntegrationTestConfig.java,AttachmentEndpointIntegrationTest.java,ResponseSubmissionIntegrationTest.java,TcpCommandIntegrationTest.java,Integration Test Contract.mdThe server validates all device/entity IDs with
Guid.TryParse(). Test IDs like "inttest-00000000-..." and "inttest-attachment-001" were being rejected as invalid GUIDs. All test IDs are now valid UUIDs (e.g.,00000001-0000-0000-0000-000000000001). The Integration Test Contract documentation was updated to use these valid UUIDs across all examples and seed data specifications.3. MaterialDto serialisation fix
Files:
MaterialDto.java@SerializedName("Type")was changed to@SerializedName("MaterialType")to match the server'sAndroidMaterialDto.csand Validation Rules §2A(1)(a). Without this fix, the material type field deserialised asnull.4. Roboelectric compatibility fixes
Files:
NetworkIntegrationHarness.java,PairingManager.java,TcpSocketManager.javaThree issues caused test failures under Roboelectric's PAUSED looper mode:
ConnectivityManager.isConnected(), which always returnsfalseunder Roboelectric. Removing it allows HTTP requests to proceed normally in tests.Handler.post(), but runnables never execute in PAUSED mode. The harness now creates aHandlerthat overridessendMessageAtTimeto dispatch runnables immediately.LiveData.getValue()returns stale/null values under Roboelectric because observers don't dispatch. Avolatilefield is now written alongsideLiveData.postValue()and read bygetCurrentState()for thread-safe, immediate access in tests.5. Server Integration mode
Files:
Program.cs(modified),IntegrationSeedService.cs(new),IntegrationResetController.cs(new),appsettings.Integration.json(new),launchSettings.json(modified)Implemented the full Integration environment per Integration Test Contract §12:
IntegrationSeedServiceseeds the required device config, materials, questions, attachments, and feedback on startup./api/v1/integration/resetclears all data and re-seeds, called by the test harness between test runs.Integrationenvironment bypasses the startup orphan cleanup logic intended for production.Integrationprofile tolaunchSettings.jsonto simplify running the server with the correct environment and port configuration.6. TCP connection cleanup (flaky test fix)
Files:
ITcpPairingService.cs,TcpPairingService.cs,IntegrationResetController.csThe
handRaised_receivesHandAcktest was intermittently flaky (~1 in 4 full-suite runs) because the server retained TCP connection state from previous tests. AddedDisconnectAllClients()toITcpPairingServiceand its implementation inTcpPairingService, which:TcpClientconnections_connectedClients,_deviceConnections,_lastHeartbeat,_pendingMaterials,_pendingFeedbacks,_pendingLockUnlockCalled as step 0 of the reset controller before clearing database state.
7. GUID validation — return 404 instead of 400/403
Files:
AttachmentController.cs,ConfigController.cs,DistributionController.cs,FeedbackController.csAll 4 controllers previously returned
400 BadRequestfor invalid GUID format, andConfigControllerreturned403 Forbiddenfor non-Android devices. Changed all to return404 NotFoundwith generic messages (e.g., "No materials available for this device"). This avoids leaking information about ID validity to unauthenticated callers and aligns with RESTful practice for resource-oriented endpoints.8. Windows unit test alignment
Files:
AttachmentControllerTests.cs,ConfigControllerTests.cs,DistributionControllerTests.cs,FeedbackControllerTests.csUpdated 7 test methods across 4 files to expect
NotFoundObjectResultinstead ofBadRequestObjectResult/403 Forbidden, matching the controller changes above. Test method names were updated accordingly (e.g.,Returns400BadRequest->Returns404NotFound).9. Comprehensive Integration Tests
Files:
ResponseLifecycleIntegrationTest.java,AttachmentDownloadIntegrationTest.java,ConfigFieldsIntegrationTest.java,DistributionDataIntegrationTest.java,FeedbackDataIntegrationTest.javaAdded five new integration test suites to ensure robust end-to-end functionality and strict adherence to the API Contract:
How to verify
Run the Windows server in Integration mode
Use the new launch profile for one-step startup:
cd windows/ManuscriptaTeacherApp/Main dotnet run --launch-profile IntegrationThis starts the server with:
Run Android integration tests against the server
With the server running in Integration mode on the same machine or network:
To point at a remote server, set environment variables before running: