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

Add compatibility with selenium greater than 3.X #632

Merged
merged 2 commits into from
Apr 4, 2018

Conversation

jlamaille
Copy link
Contributor

No description provided.

Copy link
Member

@bsideup bsideup left a comment

Choose a reason for hiding this comment

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

Hi @jlamaille!

Thanks for this fix! The only thing I miss is a test. Do you think you can add it? Do you need help?

@jlamaille
Copy link
Contributor Author

Hi @bsideup
I had thought about it but the mock part of the classpath seemed a little complex.
I can add TUs on the reading part of the inputStream, it could already be pretty good ?

@bsideup
Copy link
Member

bsideup commented Apr 4, 2018

@jlamaille yeah, I think you can just extract getVersionFromManifest(Manifest manifest) to avoid IO and focus on the business logic part of it

@jlamaille
Copy link
Contributor Author

That's exactly what I planned to do.
As soon as I find a little time, I push that.

@jlamaille
Copy link
Contributor Author

I added 2 unit tests for detection of Selenium versions above / below 3

@bsideup bsideup added this to the next milestone Apr 4, 2018
@bsideup bsideup merged commit 0abaf4a into testcontainers:master Apr 4, 2018
@bsideup
Copy link
Member

bsideup commented Apr 4, 2018

@jlamaille merged, thanks! 💪

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.

None yet

2 participants