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

Make default file recording directory of BrowserWebDriverContainer platform independent by kiview · Pull Request #2562 · testcontainers/testcontainers-java · GitHub

/ testcontainers-java Public

Make default file recording directory of BrowserWebDriverContainer platform independent - #2562

Merged
kiview merged 4 commits into
masterfrom
vnc-windows-defaultdir-fix
Apr 13, 2020
ConversationCommits (4)ChecksFiles changed
Merged

Make default file recording directory of BrowserWebDriverContainer platform independent#2562
kiview merged 4 commits into
masterfrom
vnc-windows-defaultdir-fix

Conversation

kiview commented Apr 13, 2020

Copy link
Copy Markdown
Member

Fixes incompatibility of Windows with default recording directory, makes Windows work OOTB.

Not 100% sure how to best handle the IOException in the constructor.

kiview added this to the next milestone Apr 13, 2020
kiview requested review from bsideup and rnorth as code owners April 13, 2020 07:57
bsideup reviewed Apr 13, 2020
View reviewed changes
Comment thread modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java Outdated
.withStrategy(new HostPortWaitStrategy())
.withStartupTimeout(Duration.of(15, SECONDS));

try {

bsideup Apr 13, 2020

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

IMO we should move it to configure and create the dir only if recording is enabled

kiview Apr 13, 2020

Copy link
Copy Markdown
Member Author

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

Moved the code.

bsideup commented Apr 13, 2020

Copy link
Copy Markdown
Member

/azp run "Windows 10 - Docker for Windows"

azure-pipelines Bot commented Apr 13, 2020

Copy link
Copy Markdown
No pipelines are associated with this pull request.

bsideup commented Apr 13, 2020

Copy link
Copy Markdown
Member

/azp run Windows 10 - Docker for Windows

azure-pipelines Bot commented Apr 13, 2020

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

kiview commented Apr 13, 2020

Copy link
Copy Markdown
Member Author

/azp run Windows 10 - Docker for Windows

azure-pipelines Bot commented Apr 13, 2020

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

bsideup reviewed Apr 13, 2020
View reviewed changes
Comment thread modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java Outdated
private static final int VNC_PORT = 5900;

private static final String NO_PROXY_KEY = "no_proxy";
public static final String TC_TEMP_DIR_PREFIX = "tc";

bsideup Apr 13, 2020

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

Private?

kiview Apr 13, 2020

Copy link
Copy Markdown
Member Author

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

Of course, sorry.

bsideup commented Apr 13, 2020

Copy link
Copy Markdown
Member

/azp run Windows 10 - Docker for Windows

azure-pipelines Bot commented Apr 13, 2020

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

kiview merged commit d33f647 into master Apr 13, 2020
delete-merged-branch Bot deleted the vnc-windows-defaultdir-fix branch April 13, 2020 15:39

rnorth commented Apr 13, 2020

Copy link
Copy Markdown
Member

This was released in https://github.com/testcontainers/testcontainers-java/releases/tag/1.14.0 🎉

Thanks for the contribution!

srempfer mentioned this pull request Apr 14, 2020
Merged
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
Make default file recording directory of BrowserWebDriverContainer pl…
ea2052c 
…atform independent (testcontainers#2562)

Co-authored-by: Sergei Egorov <bsideup@gmail.com>
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 approved these changes
rnorth Awaiting requested review from rnorth

Assignees

No one assigned

Labels

client/docker-for-windows modules/selenium type/bug

Projects

None yet

Milestone

1.14.0

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL