Skip to content

Feature/test probe video2 failing#307

Merged
bramp merged 2 commits intobramp:masterfrom
Euklios:feature/testProbeVideo2-failing
Mar 8, 2024
Merged

Feature/test probe video2 failing#307
bramp merged 2 commits intobramp:masterfrom
Euklios:feature/testProbeVideo2-failing

Conversation

@Euklios
Copy link
Copy Markdown
Collaborator

@Euklios Euklios commented Mar 5, 2024

Some systems don't allow the non asci letter é in file-names.
This results in the testProbeVide2 test failing, as the related file can't be found.
This commit renames the file, in order to allow the test to pass.

The filename wthin the ffmpeg output is not altered, to keep testing the parsing of such names

We're still checking against the 'é' symbol.
The resource filename has been renamed, as some systems can't deal with the symbol and therefore produce NPE.
@bramp
Copy link
Copy Markdown
Owner

bramp commented Mar 5, 2024

I actually had a non standard file name, to ensure the library was correctly encoding the filename strings.

Which platform is this broken on? And can we instead fix it to work?

@Euklios
Copy link
Copy Markdown
Collaborator Author

Euklios commented Mar 8, 2024

@bramp
The error occurred when running tests in a Debian-based docker image using the POSIX locale.
But I've also encountered this issue before with an arch-based installation.

Testing for special characters is reasonable. However, this PR is not concerned with the code itself.
On systems that don't support this particular character, the git clone will produce a fixture that doesn't match the assertion.
Therefore, the fixture can't be loaded at all.
We're still asserting the correct parsing of the ffprobe command output; however, we assure that the fixture can be loaded regardless of the system.

The revert in commit c4c5e1b makes sure the test still handles special symbols.

@Euklios
Copy link
Copy Markdown
Collaborator Author

Euklios commented Mar 8, 2024

As a reference, on a system with posix locale, the produced filename is: ffprobe-Always On My Mind [Program Only] - Adel'$'\303\251''n.mp4. Java uses its own locale, so the string in the code still expects ffprobe-Always On My Mind [Program Only] - Adelén.mp4.

This PR is only concerned with not testing OS behaviour we can't control

@bramp
Copy link
Copy Markdown
Owner

bramp commented Mar 8, 2024

So this sounds like the file system doesn't support the character. Which I find very surprising in this day and age of UTF-8.

I of course don't want to test the OS, but I do want to test we interact with the OS correctly. But ok, I see the changes to the test, are really just fixing the test, not breaking the behaviour of reading non-ascii named media.

@bramp bramp merged commit ee8e399 into bramp:master Mar 8, 2024
@Euklios Euklios deleted the feature/testProbeVideo2-failing branch March 11, 2024 22:16
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

Successfully merging this pull request may close these issues.

2 participants