Page MenuHomeFreeBSD

sort: Make NetBSD sort tests compatible with sort.
ClosedPublic

Authored by cyril_freebsdfoundation.org on May 11 2021, 6:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 10, 6:25 PM
Unknown Object (File)
Sun, Mar 10, 6:25 PM
Unknown Object (File)
Sun, Mar 10, 6:25 PM
Unknown Object (File)
Sun, Mar 10, 6:25 PM
Unknown Object (File)
Sun, Mar 10, 6:18 PM
Unknown Object (File)
Thu, Mar 7, 8:46 PM
Unknown Object (File)
Jan 20 2024, 1:56 AM
Unknown Object (File)
Jan 15 2024, 8:33 PM
Subscribers

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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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.

913

This actually looks the same as PR 255798?

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

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
913

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