FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add compatibility with selenium greater than 3.X by jlamaille · Pull Request #632 · testcontainers/testcontainers-java · GitHub

/ testcontainers-java Public

Add compatibility with selenium greater than 3.X - #632

Merged
bsideup merged 2 commits into
testcontainers:masterfrom
jlamaille:master
Apr 4, 2018
ConversationCommits (2)ChecksFiles changed
Merged

Add compatibility with selenium greater than 3.X#632
bsideup merged 2 commits into
testcontainers:masterfrom
jlamaille:master

Conversation

jlamaille commented Apr 2, 2018

Copy link
Copy Markdown
Contributor

No description provided.

bsideup requested changes Apr 4, 2018
View reviewed changes

bsideup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Apr 4, 2018

Copy link
Copy Markdown
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 commented Apr 4, 2018

Copy link
Copy Markdown
Member

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

jlamaille commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

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

jlamaille commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

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

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

bsideup commented Apr 4, 2018

Copy link
Copy Markdown
Member

@jlamaille merged, thanks! 💪

jlamaille mentioned this pull request Apr 5, 2018
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bsideup bsideup requested changes
kiview Awaiting requested review from kiview
rnorth Awaiting requested review from rnorth

Assignees

No one assigned

Labels

modules/selenium type/bug

Projects

None yet

Milestone

1.7.0

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL