Skip to content

Conversation

@ordian
Copy link
Contributor

@ordian ordian commented Aug 3, 2020

@ordian ordian changed the title travis: pin geckodriver and enable chrome tests travis: pin geckodriver in wasm-pack tests Aug 3, 2020
@ordian ordian marked this pull request as ready for review August 3, 2020 16:25
@ordian ordian requested review from dvdplm and niklasad1 August 3, 2020 16:25
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

I read through the linked tickets, and I see the pin to geckodriver@1.19.1 which I assume uses geckodriver 0.26 under the hood somewhere, but I'm not sure what to make of the rest of the changes. How is this related to chromedriver which is now removed?

@ordian
Copy link
Contributor Author

ordian commented Aug 3, 2020

pin to geckodriver@1.19.1 which I assume uses geckodriver 0.26 under the hood somewhere

correct

I'm not sure what to make of the rest of the changes. How is this related to chromedriver which is now removed?

chromedriver is needed for --chrome tests, which we don't do anyway due to https://travis-ci.org/github/paritytech/parity-common/jobs/714472822#L2348 failing on Linux

@dvdplm
Copy link
Contributor

dvdplm commented Aug 4, 2020

I'm not sure what to make of the rest of the changes. How is this related to chromedriver which is now removed?

chromedriver is needed for --chrome tests, which we don't do anyway due to https://travis-ci.org/github/paritytech/parity-common/jobs/714472822#L2348 failing on Linux

I still don't get it, sorry! Did both chrome and gecko start to fail and with this PR we fix only gecko? Is there hope to fix chrome as well or are you saying it's impossible or not needed, or…?

@ordian
Copy link
Contributor Author

ordian commented Aug 4, 2020

Did both chrome and gecko start to fail and with this PR we fix only gecko? Is there hope to fix chrome as well or are you saying it's impossible or not needed, or…?

I didn't delete --chrome flag in this PR, it wasn't present because it doesn't work and I don't have time to investigate why, so it is out of scope of this PR. This PR only fixes --firefox tests to make CI green. And removes currently unused dependencies.

@dvdplm
Copy link
Contributor

dvdplm commented Aug 4, 2020

This PR only fixes --firefox tests to make CI green. And removes currently unused dependencies.

Now I finally get it. We have cd kvdb-web/ && wasm-pack test --headless --firefox but that broke with the latest release. We were installing chromedriver as well, but never had a wasm-pack test --headless --chrome?

Thanks.

@dvdplm dvdplm merged commit 0431acb into master Aug 4, 2020
@dvdplm dvdplm deleted the ao-workaround-for-geckodriver-bug branch August 4, 2020 13:14
@ordian
Copy link
Contributor Author

ordian commented Aug 4, 2020

We have cd kvdb-web/ && wasm-pack test --headless --firefox but that broke with the latest release. We were installing chromedriver as well

exactly

but never had a wasm-pack test --headless --chrome?

We used to have it until it started to fail: #324. PRs to bring it back are welcome of course ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants