fix(#320): record by test with @ClassRule for BrowserWebDriverConta…

[javathought]

…iner

alter 3 classes
add a test watcher (ContainerWatcher)
to be run with a container with ps command to enable stopping recording on end of test

[javathought]

A test has to be annotated by these 2 rules to have a recording by test method.
If there's no watcher, a single recording for the Test class is created.

    @ClassRule
    public static BrowserWebDriverContainer chrome = new BrowserWebDriverContainer()
            .withDesiredCapabilities(DesiredCapabilities.chrome())
            .withRecordingMode(BrowserWebDriverContainer.VncRecordingMode.RECORD_ALL, new File("target"));

    @Rule
    public ContainerWatcher watcher = new ContainerWatcher(chrome);

I also created an image that can stop recording at end each test as it needs ps command and the original image does not have it. Simply add this line in testcontainers.properties.

vncrecorder.container.image=javathought/vnc-recorder:latest

[bsideup]

You're trying to "optimize" things by not starting a new VncRecorder container for every recording.
But actually running exec's, killing old binary and other things might lead to the same performance or even worse.

Have you compared your solution with "recorder per test" I recommended you before?

[bsideup]

there is already .exec method, why do we need another one?

[javathought]

I'll check it

[javathought]

Do you mean execInContainer? It's blocking and waiting for the end of the call.
I need one to launch recording and continue testing while recording is running

[bsideup]

what does this method do?

[javathought]

the method is called by the watcher, which is created with a GenericContainer. In BrowserWebDriverContainer, the implementation start recording in a file

[javathought]

I've done some tests.

• Using @ClassRule for BrowserWebDriverContainer is much faster just @rule.
• Killing old binaries is faster than letting several recording on the container.

I've begun comparing your solution with this. On the tests I've written, the times are similar but I have only 2 tests at the moment, perhaps a little faster with my code. But I see a difference :

• Times are almost the same on the 2 tests with your solution
• It seems to have a better time on the second time in my case
  so, on many tests, a greater difference could appear

[bsideup]

this will break things and not how JUnit expects rules to work.
I would rather add "restart" to the VncRecorder

[javathought]

I've made a comparison with 4 tests in a class :

• with the PR, time is between 17 s. and 20 s.
• with your solution, time is between 20 s. and 22 s.

[javathought]

@bsideup There's a missing feature with your solution :

• identify failing to test to record only related videos (or at least identify this videos in the name)

[bsideup]

@javathought here is how you can add this "feature" :)

static File recordingDir = new File("build/recording-" + System.currentTimeMillis());

@ClassRule
public static BrowserWebDriverContainer chrome = new BrowserWebDriverContainer<>()
        .withDesiredCapabilities(DesiredCapabilities.chrome())
        .withNetwork(Network.SHARED)
        .withNetworkAliases("vnchost")
        .withRecordingMode(BrowserWebDriverContainer.VncRecordingMode.SKIP, null);

@Rule
public VncRecordingContainer vnc = new VncRecordingContainer(chrome) {
    @Override
    protected void failed(Throwable e, Description description) {
        saveRecordingToFile(new File(recordingDir, description.getMethodName() + ".flv"));
        super.failed(e, description);
    }
};

[javathought]

@bsideup
Thanks, it works and since times seems equivalents, I'll use it.

I want to do this also with Cucumber, so I'll test it.

[bsideup]

@javathought don't get me wrong - I want this use case to be covered, just let's find the best solution with minimum code changes :)

[kiview]

@javathought If your use case is covered now, is it okay for you, if we close this PR for now?

[javathought]

@kiview yes it's ok.
I was also able to use Cucumber with some custom classes in my tests to reproduce the same behaviour on scenarios.

I think that an update on ticket #320 with last code sample of @bsideup will help other peoplee too, as it was the origin of this.

[kiview]

@javathought Alright, I'll add the example and close the issue, thanks a lot for your participation in solving this.