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

Enable easier configuration of recording file name formatting #500

Closed
ldeck opened this issue Nov 21, 2017 · 4 comments
Closed

Enable easier configuration of recording file name formatting #500

ldeck opened this issue Nov 21, 2017 · 4 comments

Comments

@ldeck
Copy link
Contributor

ldeck commented Nov 21, 2017

When using scalatest I'd like to be able to use the test Description.getDisplayName (perhaps with a date and/or failed/passed string) to make it easier to find the relevant file for a test.

Currently, BrowserWebDriverContainer#stopAndRetainRecording is private and we can only access currentVncRecordings via reflection if we override BrowserWebDriverContainer#failed, for example.

I'd like to suggest the following refactor:

private void stopAndRetainRecording(Description description) {
    File recordingFile = new File(vncRecordingDirectory, "recording-" + filenameDateFormat.format(new Date()) + ".flv");
    ....
}

becomes:

protected File toRecordingFile(File vncRecordingDirectory, Description description, boolean successful) {
    return new File(vncRecordingDirectory, "recording-" + filenameDateFormat.format(new Date()) + ".flv");
}
private void stopAndRetainRecording(Description description, boolean successful) {
    File recordingFile = toRecordingFile(vncRecordingDirectory, description, successful);
    ....
}

Naturally, failed and succeeded would pass along the relevant flag.

I'm happy to create a pull-request with these changes with any suggested alterations to the above.

@ldeck
Copy link
Contributor Author

ldeck commented Nov 21, 2017

Alternative refactor:

public BrowserWebDriverContainer() {
    ....
    withRecordingFileFormatter((description, succeeded) -> new File(vncRecordingDirectory, "recording-" ...);
}
public SELF withRecordingFileFormatter(BiFunction<Description, Boolean, File> recordingFileFormatter) {
        this.recordingFileFormatter = recordingFileFormatter;
        return self();
}
private void stopAndRetainRecording(Description description, boolean successful) {
    File recordingFile = recordingFileFormatter.apply(description, successful);
    ....
}

@rnorth
Copy link
Member

rnorth commented Nov 26, 2017

@ldeck good idea - I'd certainly be in favour of the filenames being easier to identify (by default!)

I think your second suggested alternative looks good as an approach for customisation, but if you have time to create a PR would you mind also changing the default filename formatter? Something like this might be useful: FAILED-ClassNameOfTest-methodNameOfTest-20171126-115603.flv. WDYT?

ldeck added a commit to ldeck/testcontainers-java that referenced this issue Nov 27, 2017
By default this returns a file in the given vlc recording directory named '<RESULT>-<TestClassName>-<testMethodName>-<YYYYMMdd-HHmmss>.flv', where RESULT is either 'PASSED' or 'FAILED'.

A custom factory can be provided to BrowserWebDriverContainer#withRecordingFileFactory.

The factory is only applicable if the BrowserWebDriverContainer#recordingMode enables the retention of recordings.

Fixes testcontainers#500.
@ldeck
Copy link
Contributor Author

ldeck commented Nov 27, 2017

@rnorth sounds good.

ldeck added a commit to ldeck/testcontainers-java that referenced this issue Nov 27, 2017
@ldeck
Copy link
Contributor Author

ldeck commented Dec 5, 2017

@rnorth are you happy to accept the merge or would you like anything changed?

ldeck added a commit to ldeck/testcontainers-java that referenced this issue Dec 6, 2017
ldeck added a commit to ldeck/testcontainers-java that referenced this issue Dec 9, 2017
ldeck added a commit to ldeck/testcontainers-java that referenced this issue Dec 9, 2017
ldeck added a commit to ldeck/testcontainers-java that referenced this issue Dec 9, 2017
ldeck added a commit to ldeck/testcontainers-java that referenced this issue Dec 9, 2017
rnorth pushed a commit that referenced this issue Dec 9, 2017
Add RecordingFileFactory with default implementation

By default this returns a file in the given vlc recording directory named '<RESULT>-<TestClassName>-<testMethodName>-<YYYYMMdd-HHmmss>.flv', where RESULT is either 'PASSED' or 'FAILED'.

A custom factory can be provided to BrowserWebDriverContainer#withRecordingFileFactory.

The factory is only applicable if the BrowserWebDriverContainer#recordingMode enables the retention of recordings.

Fixes #500.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants