-
Notifications
You must be signed in to change notification settings - Fork 425
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
firefox tests are failing on windows #770
Comments
looking |
When running Logs
|
i think it might be related to the changes made in this pr (but not necessarily introduced): #706 this issue was opened with the complaint that CI could not run tests with the headless drivers with #706 referenced as a possible solution: rustwasm/wasm-pack-template#42 |
When using the Rust Webpack Template, it does successfully work with geckodriver, so wasm-pack 0.8.1 still works:
|
Using wasm-pack from |
interesting - over at cloudflare/wrangler-legacy#1011 we're having issues with our azure pipeline windows image as well |
@Pauan shared this with me on discord... appveyor/ci#3286 i'm gonna make a PR to update our binary versions since we should do that anyways, maybe it will help? i don't actually feel that confident if wrangler is also struggling, but it's worth a try |
in the process of updating the hardcoded binary versions, i ended up reading the release notes for geckodriver v0.26.0 and looks like this could be a culprit? https://github.com/mozilla/geckodriver/releases/tag/v0.26.0 |
ok so i think it's this: rustwasm/wasm-bindgen@95ab24a#diff-df264b96f2407d9b6155871a732eff96R16 either way, im going to move our CI to azure, i think wasm-bindgen has solved this issue and i'm gonna crib their config. we;ve wanted to move anyways. |
ok so it's actually what i said before, as better elaborated in this issue, which i didn't read enough of last time: mozilla/geckodriver#1617 tl;dr geckodriver v0.24.0 was built with a new build pipeline that introduced a new runtime dep i think we have 2 options,
version 3 seems... best for now? don't love the options, but it is what it is |
The VC requirement is temporary and they will fix it soon. So until then, I think we should downgrade. I don't think we should be telling users to install a dep which will be unneeded in the near future, that just bloats up their computer for no real gain. |
@fitzgen @drager @alexcrichton ^ any thoughts? i'm ok with @Pauan's suggestion- would like to have a decision by tomorrow so we can get this out, so lemme know if you have a different suggestion :) |
https://ci.appveyor.com/project/ashleygwilliams/wasm-pack-071k0/builds/30355575 dropping version may not actually solve the issue :/ |
nevermind i misunderstood the logic of how the geckodriver was being installed(it attempts to update itself so the default value is not useful.) i'll be hard coding the version for windows users. |
the fix here is actually easier than expected, i'm preventing the geckodriver from attempting to self-update on windows. i think that's the right call here since it's actually the currently released behavior (the auto-update was introduced after the last release.) |
wrong again, it's true that it would self update, so it's good i changed that but the default gecko driver pre installed on the CI is 0.26.0 so we'll need to identify that and then override it to get CI to work. |
I agree with @Pauan :) |
doesn't appear to line up with a change in wasm-pack but i'm genuinely not sure. i feel like this only started very recently, past few days at best- thoughts @EverlastingBugstopper ?
The text was updated successfully, but these errors were encountered: