Skip to content
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

Create a web driver tunnel #73

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edhager
Copy link
Contributor

@edhager edhager commented Mar 12, 2019

This PR creates a new Intern tunnel that runs the various browsers' web driver servers without using Selenium or Java.

Fixes #54

@edhager edhager requested a review from jason0x43 March 12, 2019 19:18
@edhager edhager self-assigned this Mar 12, 2019
@edhager
Copy link
Contributor Author

edhager commented Mar 12, 2019

To do list:

  • Add Edge support
  • Add IE support
  • Write unit tests
  • Refactor Selenium tunnel to use new web driver configs module.

Copy link
Member

@jason0x43 jason0x43 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, I just had a few suggestions and comments

package.json Outdated Show resolved Hide resolved
src/Tunnel.ts Outdated Show resolved Hide resolved
src/Tunnel.ts Outdated Show resolved Hide resolved
src/WebDriverProxy.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,297 @@
import * as express from 'express';
import * as request from 'request';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use request from @theintern/common since that's already a dep?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to refactor my proxying function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored the code to use request from @theintern/common. I created a PR, theintern/common#4, for a small change to common.

src/WebDriverTunnel.ts Show resolved Hide resolved
src/tunnelChildProcesses.ts Outdated Show resolved Hide resolved
src/webDriverConfig.ts Outdated Show resolved Hide resolved
package.json Outdated
@@ -18,6 +18,7 @@
"@theintern/dev": "~0.9.0",
"@types/decompress": "~4.2.2",
"@types/node": "~9.6.11",
"@types/request": "^2.48.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this anymore

@edhager edhager force-pushed the issue-54-webdriver-binaries-config branch 2 times, most recently from e99e90e to 5f12312 Compare May 15, 2019 01:58
@edhager edhager force-pushed the issue-54-webdriver-binaries-config branch 3 times, most recently from 1f9f612 to 2a145c2 Compare November 12, 2019 00:09
@edhager edhager force-pushed the issue-54-webdriver-binaries-config branch from 2a145c2 to 7ef2c94 Compare November 12, 2019 00:11
@edhager edhager removed their assignment Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants