Skip to content

Enable easier configuration of recording file name formatting #500

Closed

Description

@ldeck

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.

Activity

  1. ldeck commented on Nov 21, 2017

    @ldeck
    ContributorAuthor

    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);
        ....
    }
  2. rnorth commented on Nov 26, 2017

    @rnorth
    Member

    @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?

  3. added a commit that references this issue on Nov 27, 2017
    b08541e
  4. ldeck commented on Nov 27, 2017

    @ldeck
    ContributorAuthor

    @rnorth sounds good.

  5. added a commit that references this issue on Nov 27, 2017
    30dcbb7
  6. ldeck commented on Dec 5, 2017

    @ldeck
    ContributorAuthor

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

  7. added 5 commits that reference this issue on Dec 6, 2017
    0798524
    4462951
    b42082f
    bb28bed
    e014048
  8. added a commit that references this issue on Dec 9, 2017
    6620d23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions