serverless-offline
serverless-offline copied to clipboard
WebSocket Integration Tests (Take 2)
This PR replaces #778
Currently, WebSocket has 35 tests that are not part of npm run test.
This PR will make the WebSocket tests be part of the regular testing flow.
To Do:
- [x] Remove dependancy on running
npm linkin order use current branchserverless-offlinein testing. - [x] Remove dependancy on running local DynamoDB instance for testing.
- [x] Move from
__test__/manual/websocketto__test__/integration/websocket. - [x] Use
jestinstead ofmochaas a testing platform. - [x] Run al least one test.
- [x] Add all
maintests. - [x] Add all
RouteSelectiontests. - [x] Add all
authorizertests. - [x] Remove Dynamo DB dependancy.
hey @computerpunc !! thank you so much!! let me know when this is ready to be pulled in!
@dnalborczyk
This PR is ready for review. All 35 tests pass on AWS. Before merging from master all unskipped tests passed locally but, for some reason, after merging with latest master they do not.
Can you have a look to understand why no WebSocket test passes?
EDIT: alpha.40 seems to work fine but things break in alpha.41. When running serverless-offline.alpha41, the following error appears:
Error --------------------------------------------------
Error: Invalid route options ( ) "value" must be an object
at Object.exports.apply (.../serverless-offline-websocket-tests/node_modules/@hapi/hapi/lib/config.js:19:15)
at new module.exports.internals.Route (/.../serverless-offline-websocket-tests/node_modules/@hapi/hapi/lib/route.js:35:16)
at internals.Server._addRoute (.../serverless-offline-websocket-tests/node_modules/@hapi/hapi/lib/server.js:485:23)
at internals.Server.route (.../serverless-offline-websocket-tests/node_modules/@hapi/hapi/lib/server.js:478:22)
at HttpServer.start (.../serverless-offline-websocket-tests/node_modules/serverless-offline/dist/index.bb3aea00.js:146:18)
at ApiGatewayWebSocket.start (.../serverless-offline-websocket-tests/node_modules/serverless-offline/dist/index.bb3aea00.js:578:28)
at ServerlessOffline.start (.../serverless-offline-websocket-tests/node_modules/serverless-offline/dist/index.js:102:39)
at processTicksAndRejections (internal/process/task_queues.js:93:5)
at ServerlessOffline._startWithExplicitEnd (.../serverless-offline-websocket-tests/node_modules/serverless-offline/dist/index.js:126:5)
@computerpunc thank you! sure, I'll have a look.
@computerpunc just pulled down the PR as-is, and tests appear to run. is this branch based on alpha 40?
Test Suites: 1 skipped, 23 passed, 23 of 24 total
Tests: 1 skipped, 237 passed, 238 total
Snapshots: 4 passed, 4 total
Time: 17.62s
update:
in order to make 'em fail, I have to rebase alpha 41 (or head of master) I assume?
@computerpunc just pulled down the PR as-is, and tests appear to run. is this branch based on
alpha 40?Test Suites: 1 skipped, 23 passed, 23 of 24 total Tests: 1 skipped, 237 passed, 238 total Snapshots: 4 passed, 4 total Time: 17.62supdate: in order to make 'em fail, I have to rebase
alpha 41(or head of master) I assume?
@dnalborczyk
I ran the regular old (i.e., not based on this PR) WebSocket tests on 2 computers and the WebSocket tests pass with alpha.40 but fail with alpha.41. So we can assume that on my computers something broke with the changes entered after alpha.40 and before alpha.41.
This PR was based on 5eca9cfe415b5ff698595ca2ebbb89c180688a26 which was published on Sep. 16 after alpha.40 but before alpha.41.
Each time I tried afterwards to merge with master the tests failed locally so I aborted.
On 8b54134e3cd719a37760b631c83d77995a196134 you merged for the first time master into this PR.
It's interesting because this PR never passed CI as for the same reason I see locally.
If it's running without any problem on your side, how about you merge this PR (via a branch?) into master, publish alpha.42 and I'll continue the tests on my end from this point?
@dnalborczyk
In any case, I found the bug:
In src/events/websocket/HttpServer.js
change
const routes = [...connectionsRoutes(this._webSocketClients), catchAllRoute]
to
const routes = [...connectionsRoutes(this._webSocketClients), catchAllRoute()]
Hope this helps and you can add the WebSocket tests soon.
@dnalborczyk
In any case, I found the bug:
In
src/events/websocket/HttpServer.jschangeconst routes = [...connectionsRoutes(this._webSocketClients), catchAllRoute]toconst routes = [...connectionsRoutes(this._webSocketClients), catchAllRoute()]Hope this helps and you can add the WebSocket tests soon.
ah, nice. thank you! I'll have a look now!
@computerpunc just a quick update. applying the folder name changes locally and merge your PR-branch into master. If for some reason the tests don't pass we can fix it in master.
@computerpunc just a quick update. applying the folder name changes locally and merge your PR-branch into master. If for some reason the tests don't pass we can fix it in master.
@dnalborczyk Not sure I follow. Do you want me to do anything or you are going to take care of it?
2 yo PR, closing sorry, please feel free to reopen.
re-opening as a reminder to have a look.