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

Use Async WebDiver in Tests #591

Closed
5 tasks
rmorshea opened this issue Jan 15, 2022 · 4 comments · Fixed by #703
Closed
5 tasks

Use Async WebDiver in Tests #591

rmorshea opened this issue Jan 15, 2022 · 4 comments · Fixed by #703
Labels
priority-2-moderate Should be resolved on a reasonable timeline. type-ci About change and updates to the Continuous Integration
Milestone

Comments

@rmorshea
Copy link
Collaborator

rmorshea commented Jan 15, 2022

Current Situation

Currently, we run tests which rely on the browser using a synchronously executing web driver from Selenium. As a result, server implementations have been required to provide a run_in_thread method such that they can be run in the background while the blocking test code is allowed to proceed.

Unfortunately, some async servers don't play well with threads. This is primarily due to the fact that cleanly stopping a thread can be somewhat challenging. For example, IDOM is currently locked into version sanic<19.12 because later releases have been developed without consideration for threading.

Proposed Actions

As a result, we should switch to an async web driver like arsenic or pyppeteer and add a ServerType.run_async method to the various server implementations. Once this is done we can deprecate the ServerType.run_in_thread method.

Then lastly, we can upgrade Sanic to the latest version.

Work Items

  • Implement ServerType.run_async and switch to async web driver
  • Mark ServerType.run_in_thread as deprecated
  • Update downstream libraries (e.g. custom component template and anything made from it).
  • Upgrade sanic and raise when run_in_thread is called for sanic>=19.12
  • Remove ServerType.run_in_thread
@rmorshea rmorshea added the priority-2-moderate Should be resolved on a reasonable timeline. label Jan 15, 2022
@rmorshea rmorshea added this to the 2.0 milestone Jan 15, 2022
@Archmonger
Copy link
Contributor

We can also use arsenic instead of Selenium.

It would save us a lot of effort in developing custom tooling.

@Archmonger Archmonger added the type-ci About change and updates to the Continuous Integration label Feb 17, 2022
@rmorshea
Copy link
Collaborator Author

rmorshea commented Feb 18, 2022

This is somewhat more urgent as a result of: GHSA-7p79-6x2v-5h88

@Archmonger
Copy link
Contributor

Security issues are not publicly visible, and I don't have access to them.

@rmorshea
Copy link
Collaborator Author

I edited the link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-2-moderate Should be resolved on a reasonable timeline. type-ci About change and updates to the Continuous Integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants