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

Regard specified VNC recording directory for BrowserWebDriverContainer again #2574

Merged

Conversation

srempfer
Copy link
Contributor

Since the changes of #2562 which were released with 1.14.0 the possibility to specify a target directory for the VNC recordings doesn't work anymore.

@bsideup bsideup added this to the next milestone Apr 14, 2020
}, Optional.empty());

String[] files = vncRecordingDirectory.list(new PatternFilenameFilter("PASSED-" + testName + "-.*\\.flv"));
assertTrue("Recorded file not found", files.length == 1);
Copy link
Member

Choose a reason for hiding this comment

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

Excellent addition to the test, thanks.

Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

LGTM after @bsideup's proposed code style change.

@bsideup
Copy link
Member

bsideup commented Apr 14, 2020

Hi @srempfer,

Good catch! Sorry for letting it through. We thought this part is tested (it should be!) and did not bother double checking 😅

Extra points for adding a test - now it is tested :D

@srempfer srempfer force-pushed the fixVncRecordingDirectoryHandling branch from 5f90aca to 0810dab Compare April 14, 2020 11:31
@bsideup bsideup merged commit c3d37e4 into testcontainers:master Apr 16, 2020
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
…r again (testcontainers#2574)

Co-authored-by: Stefan Rempfer <srempfer@rt.ipnt.de>
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

3 participants