[DevTools] PR4: Testing #2918
[DevTools] PR4: Testing #2918
Conversation
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| undefined, // No active log pollers for this test | ||
| undefined, // No toggle start times for this test | ||
| mockPrinter, |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
| mockGetSandboxState, | ||
| mockBackendClient, | ||
| 'amplify-backend', // Default namespace | ||
| mockRegionFetcher, // Pass the mock region fetcher |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
| new Map(), | ||
| new Map(), | ||
| printer, | ||
| mockLambdaClient, // Pass the mock Lambda client |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
| undefined, // No active log pollers for this test | ||
| undefined, // No toggle start times for this test | ||
| mockPrinter, |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
| undefined, | ||
| this.logger, |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning test
| undefined, | ||
| undefined, | ||
| this.logger, |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning test
| // Simulate corrupted storage by mocking loadResources to throw an error | ||
| const originalLoadResources = storageManager.loadResources; | ||
| storageManager.loadResources = () => null; |
There was a problem hiding this comment.
Comments says "throw an error". Replacement function (which seems like maybe it should be a mock?) simply returns null.
| // Should recover by returning an empty array instead of crashing | ||
| assert.deepStrictEqual(resources, []); |
There was a problem hiding this comment.
The test description is pretty lofty -- touting "recovery" from "corruption." (Sounds heroic, actually!) But, this test doesn't really demonstrate recovery. It seems to demonstrate that a null result from loadResources equates to an empty array "return" value.
| // Set up a promise that will resolve when we receive resources | ||
| const resourcesReceived = new Promise<unknown>((resolve) => { | ||
| clientSocket.on(SOCKET_EVENTS.SAVED_RESOURCES, (data) => { | ||
| resolve(data); | ||
| }); | ||
| }); | ||
|
|
||
| // Request saved resources | ||
| clientSocket.emit(SOCKET_EVENTS.GET_SAVED_RESOURCES); |
There was a problem hiding this comment.
Seeing this socket request/response pattern makes me wish we'd invested a little time in creating a protocol to handle these details under the hood -- to make these requests look more like "normal" RPC calls.
In lieu of that, for now, can we at lest rename "response" type event names to clearly indicate that they are in fact in response to a "call". Would be interested in a naming convention proposal. E.g., a straw man:
{METHOD_NAME}_REQUEST{METHOD_NANE}_RESPONSESTREAM_{NAME}_CHUNK
Or something like that.
Various integration tests and misc tests that require previous PR3 changes to be merged. note this PR includes ONLY the test files, none of the actual files, so the tests don't actually run yet. It is merely to be able to view the tests.