Make recorder .flv videos scrollable (#512)#3180
Make recorder .flv videos scrollable (#512)#3180bsideup merged 10 commits intotestcontainers:masterfrom
Conversation
kiview
left a comment
There was a problem hiding this comment.
Thanks a lot, I have added some discussion point.
Looking forward to getting this quality of life fix merged 🙂
core/src/main/java/org/testcontainers/containers/VncRecordingContainer.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/VncRecordingContainer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/VncRecordingContainer.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
|
Some tests don't pass : due to ExpectedConditions.visibilityOfAllElementsLocatedBy(By.cssSelector("#search h3") that is not met, but when using just I will try find out why and commit a fix |
|
The tests were fixed by changing the Css Selector to look for parent element
|
4b75c5e to
f76e59a
Compare
|
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
|
I've merged from master, as we've had a problem with GitHub Actions that needed to be fixed in the master branch. |
core/src/main/java/org/testcontainers/utility/TestcontainersConfiguration.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
...s/selenium/src/test/java/org/testcontainers/junit/ChromeRecordingWebDriverContainerTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/VncRecordingContainer.java
Outdated
Show resolved
Hide resolved
rnorth
left a comment
There was a problem hiding this comment.
We had a discussion offline, and decided that we can't risk changing the default emitted file format without warning.
I'll take an action to amend this PR, adding a parameter for format which will default to FLV. In 1.16.0 we'll change the default to MP4.
Sorry that this came as a late decision.
|
@rnorth sounds good.
Just note that it does not suffice to change the extension |
…RecordingWebDriverContainerTest.java
|
Thanks everyone for letting me contribute to this awesome project, hope it will be useful to the community 😃 |
|
@oussamabadr thank you so much for contributing it! 💯 Expect it to be released very soon :) |
1.16.0 got released. However I was unable to spot the change in the release notes. |
|
@AB-xdev The default is FLV, but you can specify MP4 as well. |
Thank you for the ultra-fast feedback 👍 Backstory: |
|
@AB-xdev we forgot to switch the default in 1.16.0, sorry 😅 |
|
No problem. Just a question: Do you still want to switch or not? |
|
@AB-xdev we do! It is the right default value and will make many users' lives easier, just we need to do it in a "major" release, to not confuse our users. So, 1.17.0 then 😅 |
I have written an implementation to fix the scrollable video using ffmpeg command, which will also reduce the video size as mentioned by @leonard84
I have also opened a related pull Request testcontainers/vnc-recorder#4 by adding ffmpeg to the docker image, which (I saw) necessary to avoid using another container and also simplify the fix.
Just to mention that some test won't pass untill the new vnc-recorder docker image is available with tag 1.2.0.
(Thank to @kiview for his assitance during hack-commit-push (last june) which make contributing to testcontainers more clear for me.)
Fixes #512.