-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
testkit-backend: Add support for testing driver in browser #798
Merged
Conversation
This file contains 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
bigmontz
changed the title
testkit-backend: Separate the socket connection, protocol and controller
testkit-backend: Decoupling socket connection, protocol and controller/request handler
Oct 26, 2021
bigmontz
force-pushed
the
4.4-new-backend
branch
from
October 29, 2021 09:52
4803f6d
to
f482649
Compare
The logic of reading and writing in the socket, call the handlers and parse the messages were mixed in the Backend class. This makes difficult to add different protocols, socket implementation or controllers to the backend. The goal of this change is separate this three concerns in `SocketServer`, `Controller` and `Protocol` with the `Backend` class as the glue between these concepts. New implementations of the `Controller` could enable this backend to test the driver running in the browser without have to change the logic of parsing the messages between testkit and testkit-backend, for instance.
bigmontz
force-pushed
the
4.4-new-backend
branch
from
November 11, 2021 12:39
f482649
to
a27edba
Compare
bigmontz
changed the title
testkit-backend: Decoupling socket connection, protocol and controller/request handler
testkit-backend: Add support for testing driver in browser
Nov 15, 2021
robsdedude
approved these changes
Nov 17, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite a few comments. None of them are a deal-breaker. Mainly typos and question.
Great PR!!!
🚀
Co-authored-by: Robsdedude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The initial implementation of the
testkit-backend
is NodeJS only which is not an issue for the time being. However, It starts to be problematic with new features being implemented test driven usingtestkit
as the source of tests. First, the time for run all the suite increased a lot with the number of tests added and the need of keeping the some integration tests to not let browser uncovered. Second, the driver running in browser was not being tested with the same rigour as in Nodejs.The following changes were done with the intent to support testkit tests in Browser.
Refactoring
The logic of reading and writing in the socket, call the handlers and parse the messages were mixed in the Backend class. This makes difficult to add different protocols, channel implementation or controllers to the backend.
The goal of this change is separate this three concerns in
Channel
,Controller
andProtocol
with theBackend
class as the glue between these concepts. New implementations of theController
could enable this backend to test the driver running in the browser without have to change the logic of parsing the messages between testkit and testkit-backend, for instance.Supporting Browser
The first step for supporting testkit in browser was creating a
web socket
server to redirect the requests to a client.RemoteController
is the class responsible for it, it acts like a proxy redirecting the requests and responses. In the other side,WebSocketChannel
is responsible for managing the messages exchanges betweenbrowser-backend
andnodejs-backend
in the browser side.The
main
function glued everything together using env variables for selecting the correct channel and controller.A bit more of structure had to be taking in place for bundle testkit for running in browser. The bundle is supported by
rollup
and defined inrollup.config.js
, this runs during the build and the outcome is stored inpublic/index.js
. The following env variables are defined during the browser build to enforce the correct configuration:TEST_ENVIRONMENT
= "LOCAL"CHANNEL_TYPE
= "WEBSOCKET"BACKEND_PORT
=WEB_SERVER_PORT
(or8000
if nothing is set inWEB_SERVER_PORT
)The last thing done during the browser build process is done by some replacements done in the
package.json
, the./src/controller/remote.js
and./src/channel/socket.js
are replaced by their interfaces since the some modules used in this files are really tricky to support in browser and they are not need at browser.After we have the browser bundle done, we need to server it. It is done by the
RemoteController
as well.At this point, you can run
TEST_ENVIRONMENT=REMOTE npm run start-testkit-backend
and open your favorite browser inlocalhost:8000
for running testkit tests in browser.Glue everything in Testkit
Last but not least, the testkit scripts were adapt to use the environment
TEST_DRIVER_BROWSER
to decide if the tests should be run in browser and in nodejs. Integration tests were segregated for running only the ones needed for the environment under tests.backend.py
is responsible for configuring correctly the backend using the env-variables and also for starting a headless browser with the browser-backend url.IMPORTANTE NOTE
This version doesn't support TLS_TESTS and STUB_TESTS since the mock servers doesn't support websocket yet