Start chromedriver with PartitionSupervisor #692
Conversation
This will open as many chromedrivers you have as cores, and partition them access to them based on your test pid. This effectively acts as a "pool" of chromedrivers without needing logic to check them in and out of the pool. It just acts as if there is a single chromedriver and it seems to just work. Related to #365
|
@AndroidOatmeal pinging you again in case the last notification got lost. |
|
@mhanberg Thank you! It's been a busy week so far, but I'll test this out in the next couple days |
|
@AndroidOatmeal no worries, I really appreciate it! |
|
@mhanberg I spent some time testing this out today and I'm happy to say it seems to work great! I can't tell you how excited I feel lol. Thanks again for the assist here. I've been running our acceptance test suite over and over with this wallaby branch trying to flush out any hiding issues. Strangely, once I got the usual |
|
Thanks for testing this out! I just returned from a work trip so I'll hopefully get this tidied up and merged in the next few days. I'll probably publish a release candidate and have more people try it out for a little bit then I'll publish the actual release. |
|
I contracted covid during the aforementioned work trip, so I haven't gotten around to this 😅. I hope to get an RC release out in the next couple days. |
|
@mhanberg Sorry to hear about the covid — it really whooped my ass too. Hope you're feeling better! |
Since we are vendoring this from the Elixir codebase, we attribute the project by providing its license in the file.
lib/wallaby/partition_supervisor.ex
Outdated
| @@ -0,0 +1,565 @@ | |||
| # Apache License | |||
There was a problem hiding this comment.
@josevalim is it okay to vendor partition supervisor like this?
There was a problem hiding this comment.
Yes, I believe you need the copyright notice and a link to the repo.
There was a problem hiding this comment.
Here it is: https://github.com/elixir-lang/elixir/blob/main/NOTICE
the second bit.
There was a problem hiding this comment.
I'll add a link, thank you! I appreciate the clarification.
Formatting has changed since we have updated the minimum Elixir version.
It seems that using `:all` with IO.binread has changed between v1.12 and v1.13, and is causing a Dialyzer problem. This patch adds some compile time logic to call it with the right argument based on the Elixir version.
This will open as many chromedrivers you have as cores, and partition
access to them based on the test pid. This effectively acts as a
"pool" of chromedrivers without needing logic to check them in and out
of the pool.
It just acts as if there is a single chromedriver and it seems to just
work.
TODO
Add the Elixir-lang license to the partition supervisor file.
Related to #365
@AndroidOatmeal do you mind testing out this branch?