fix(#320): record by test with @ClassRule for BrowserWebDriverConta…
#577
Conversation
…WebDriverContainer 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
|
A test has to be annotated by these 2 rules to have a recording by test method. I also created an image that can stop recording at end each test as it needs |
fix codacy issues
bsideup
left a comment
bsideup
left a comment
There was a problem hiding this comment.
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?
| return result; | ||
| } | ||
|
|
||
| public FrameConsumerResultCallback launchInContainer(Charset outputCharset, String... command) |
There was a problem hiding this comment.
there is already .exec method, why do we need another one?
There was a problem hiding this comment.
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
| return self(); | ||
| } | ||
|
|
||
| public void apply() { |
There was a problem hiding this comment.
the method is called by the watcher, which is created with a GenericContainer. In BrowserWebDriverContainer, the implementation start recording in a file
There was a problem hiding this comment.
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
| import org.junit.runner.Description; | ||
| import org.junit.runners.model.Statement; | ||
|
|
||
| public class ContainerWatcher extends TestWatcher { |
There was a problem hiding this comment.
this will break things and not how JUnit expects rules to work.
I would rather add "restart" to the VncRecorder
|
I've made a comparison with 4 tests in a class :
|
|
@bsideup There's a missing feature with your solution :
|
|
@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);
}
}; |
|
@bsideup I want to do this also with Cucumber, so I'll test it. |
|
@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 :) |
|
@javathought If your use case is covered now, is it okay for you, if we close this PR for now? |
|
@javathought Alright, I'll add the example and close the issue, thanks a lot for your participation in solving this. |
3 participants
…iner
alter 3 classes
add a test watcher (ContainerWatcher)
to be run with a container with
pscommand to enable stopping recording on end of test