Oops I see that I'm relying on the existence of at least some sort command; I'll fix it.

sort is a standard UNIX command and is used elsewhere, too. You can rely on its presence.

sort is a standard UNIX command and is used elsewhere, too. You can rely on its presence.

apparently it's missing inside the CI environment, because it was breaking some bats tests

BTW how do I run the test suite locally? Instead of pushing only to find I've broken more tests.

BTW how do I run the test suite locally? Instead of pushing only to find I've broken more tests.

make test

Looks like it's good now!

As new functionality, this probably warrants a test.

The problem is the CI environment doesn't have sort. I could implement a test like "if system has sort, assert sorted by version; else, assert sorted lexicographically" -- would it be ok?

The problem is the CI environment doesn't have sort.

It does have sort as it's a part of coreutils. The reason it's not found there is the path_without test function. It doesn't include sort in the list of executables it ensures to be present on PATH -- sort needs to be added to that list.

I could implement a test like "if system has sort, assert sorted by version; else, assert sorted lexicographically"

The tests need to run in systems with both "modern" and "old" sort -- so you need to mock it for the test.

Since the new logic only fires with "modern" sort, it's sufficient to mock up a "modern" sort for the new test: the logic for "old" sort is already tested in existing tests which have test data where version sorting makes no difference.

There is an existing function that mocks an executable, use it as an example. The logic for the mockup can be to just unconditionally output the correct result for the specific set of versions that you make in the test.

Ok, the test passes, but I'm not sure if that's what you meant.

If you drop the --version-sort in https://github.com/fofoni/pyenv/blob/c70edbec5386d617f5ace823c97d90a85654c51a/libexec/pyenv-versions#L134 then the test will fail. Is that it?

Also, I didn't add sort to the whitelist inside path_without, since it's not required anymore to make the tests pass.

Well, the mktemp problem is fixed.

Now, just to be sure: with the implementation we have right now, if the system doesn't have a modern sort binary, then no sort binary is used at all. In this case, what happens is the old behavior of getting the order (guaranteed by bash as you mentioned here) from the glob $versions_dir/*.

As you also already said,

Since the new logic only fires with "modern" sort, it's sufficient to mock up a "modern" sort for the new test: the logic for "old" sort is already tested in existing tests which have test data where version sorting makes no difference.

this old behavior is already checked in tests like "multiple versions", "indicates current version", "globally selected version", and others in the versions.bats file.

I understand that maybe explicitly testing the order with another test dedicated for this might be desired; I'd just like to make sure that's what you're asking for.

this old behavior is already checked in tests like "multiple versions", "indicates current version", "globally selected version", and others in the versions.bats file.

The problem is, in those tests, we don't mock anything within the ordering logic. So with a new sort, those tests would fire the new execution path rather than the old one.

Oh ok, that makes sense

Good job. Thank you!