Page MenuHomeFreeBSD

sort: Make NetBSD sort tests compatible with sort.
ClosedPublic

Authored by cyril_freebsdfoundation.org on May 11 2021, 6:42 PM.

Details

Summary

This diff primarily adds/removes flags to make the tests compatible with sort. Two tests are removed. One test is changed to expect fail due to a bug.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

contrib/netbsd-tests/usr.bin/sort/t_sort.sh
959

Rather than commenting out tests, it is better to add atf_skip calls at the beginning of each test. This reduces the number of local modifications to this file, which makes merging easier when changes are imported from NetBSD.

These are a lot of changes to an upstream (NetBSD) provided file without good annotations (please look for "Begin FreeBSD" elsewhere).

As @markj noted, making broad changes like this will definitely make my job a lot harder when I update this project. Using atf_expect_fail is an alternative to atf_skip, as it would trigger a legitimate failure if the code doesn't fail as expected.

Please put FreeBSD-specific assertions in separate test cases as well if they are FreeBSD specific.

I reduced the amount of modified lines and added comments throughout describing changes. @markj recommended that I avoid using Begin FreeBSD and End FreeBSD annotations throughout the file in order to reduce clutter.

Thanks, this looks good to me aside from the comments.

contrib/netbsd-tests/usr.bin/sort/t_sort.sh
330–332

Please add a "On FreeBSD, ..." qualifier so that it's clear that this comment isn't from upstream.

912

This actually looks the same as PR 255798?

contrib/netbsd-tests/usr.bin/sort/t_sort.sh
912

Flags for sort can come after filenames; commands like sort in -r don't treat -r as a filename. Since +0 is a valid flag, then sort in +0 shouldn't take +0 as a filename in this case.

markj added inline comments.
contrib/netbsd-tests/usr.bin/sort/t_sort.sh
912

Oof, that's weird. Looking at NetBSD's sort, they appear to stop mangling + options after encountering an option that doesn't start with - or +. We could perhaps implement that in D30234 as well. But our sort already behaves the same as GNU sort in this case, so it's probably better to keep things as they are.

This revision is now accepted and ready to land.May 12 2021, 8:26 PM

Addresses the comment issue and removes one "expect fail" due to https://reviews.freebsd.org/D30234.

This revision now requires review to proceed.May 12 2021, 8:45 PM
This revision is now accepted and ready to land.May 12 2021, 8:53 PM