I didn't bother adding tests. It doesn't seem worth having tests for
every place we use getuipathfn(). I've tested it manually.
Chatted with Raphaël to try to understand where a misunderstanding could lie here.
So to clarify this code path is never tested for in the relative case, my issue is not with this change, but with the lack of testing of these relative path altogether. Without testing of the relative case, any future code update discarding the uipathfn call would silently regress your fix.
Ah, I think I now understand where the confusion comes from. I initially said "I trust the uipathfn to work as it does elsewhere.", but that was only half the truth -- the other half is that I spotted this little bug and decided to fix it, but I don't really care much about it. Sorry I wasn't clear about that part. So I don't care enough to write tests with relative paths for theses cases. (I also don't care enough to add retroactive tests for D5916, D5936, D5937, D5939, D6264, and probably many more. And I definitely care enough to rewrite all our tests to be have more directories and for commands to be run in subdirectories, even though that would have been nice to have.) I'm happy to add tests if someone else writes them, though.
And I definitely care enough to rewrite all our tests to be have more directories and for commands to be run in subdirectories, even though that would have been nice to have.) I'm happy to add tests if someone else writes them, though.
I don't understand that part, is there some negation missing?
Since forever, the option relative path option never worked with this output. So, no user should be "surprised" if it don't. However if you fix it people may start to rely on it. Without tests it can regress, breaking user expectation.
I understant this is a side-quest cleanup for you, but I woudl rather see this slow cleanup advance twice as slow with test that at the current pace without tests.