I have tested this item ✅ successfully on 77493bd

I have tested this item ✅ successfully on 77493bd

RTC

The assumption is wrong:
-rw-r--r-- 1 root root 0 Sep 19 17:13 filewith.extentionand\behindit

This is a valid file name on posix systems...

better way maybe to use basename() before extracting the extension.

This is a valid file name on posix systems

interesting, I did not knew

seems the bot also thinks differently ?

Set back to Pending.

@SharkyKZ I'm not sure if I like the pathinfo thing. I prefer to see what is expected when reading the test. But maybe that's just my taste. Other changes look good to me.

The code looks really ugly but ok.

Whats the reason not to use pahinfo for extension extraction? Performance?

And if so please add a comment that this is the reason.

Yes, performance. But whatever maintainers decide. PR #18030 using pathinfo() has been sitting open for a while.

As a compromise between performance, readability and reliability we could use SplFileInfo::getExtension().

@SharkyKZ It's one thing if we use it in the CMS code. I would be ok with that. It's another thing if we should use it in the test, too.

I have tested this item ✅ successfully on 263bfcc

PR does what it claims to do.
I'm not a fan of using pathinfo in the test because before that change, I could see the hard-coded expected result.
But that's maybe just a matter of taste.
I've tested that the expected result is equal to the extension provided by pathinfo for all cases handled by the new test and a few more.

I'm happy with the tests and that you use phpinfo so we know at least that we do the same then php.

Could you just add the comment about performance into the code and get a second test then I will merge it.

Last change was bigger so my previous test isn't valid anymore.

@SharkyKZ Do you know where I can see in drone or appveyor if the new tests added by this PR have run?

No idea.

@HLeithner Are the tests ok now after the last change?

I don't really understand how it works. It adds a new data provider and a new test routine. But I don't see anything being added where those are called. Does that happen automatically, i.e. the test goes through test PHP files and scans for test routines and corresponding data provides, if any, according to a pre-defined naming scheme to collect which tests to run with which data?

And is there a place in logs here where I can see that they have run? Or do I have to run them in my local environment to verify that?

@SharkyKZ can you please add the comment why you used this instead of pathinfo to the code (we need much more of such comments) before the next person comes and want to change it to pathinfo

@richard67 we need a second test again maybe from @Fedik ?

The tests are executed automatically based on the name (iirc starting with test*), phpunit executes the data provider and iterates over the returned array and executes the test function with the value.

Added comment.

I have tested this item ✅ successfully on eeb2187

I have tested this item ✅ successfully on eeb2187

RTC

@HLeithner Ready for merge.

Thanks