Page MenuHomeFreeBSD

tests/ci: fix minor bugs and unskip more tests
AbandonedPublic

Authored by guest-svmhdvn on Jul 11 2025, 3:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 12, 5:53 PM
Unknown Object (File)
Sat, Oct 11, 3:36 PM
Unknown Object (File)
Sat, Oct 11, 3:36 PM
Unknown Object (File)
Sat, Oct 11, 7:07 AM
Unknown Object (File)
Wed, Oct 8, 9:25 PM
Unknown Object (File)
Wed, Oct 8, 8:27 AM
Unknown Object (File)
Fri, Oct 3, 1:55 AM
Unknown Object (File)
Tue, Sep 30, 11:25 PM
Subscribers

Details

Reviewers
lwhsu
bofh
kp
Summary

Full review of all patches in the series from bugzilla PR 288150 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288150

Siva Mahadevan (7):

tests/ci: fix missing /usr/local/{sbin,bin} in freebsdci rc PATH 
tests/ci: fix wrong chflags target path in 'beforeclean' 
tests/ci: fix unescaped kld_list var in rc.conf 
tests/ci: add KYUA_TEST_FILTERS to allow user to select specific tests 
tests/ci: add SU_CMD for privilege escalation only when necessary 
tests/ci: style: canonicalize, sort, and multiline kmods/pkgs 
tests/ci: add missing kmods and pkgs to unskip tests
Test Plan

make -DNO_ROOT CITYPE=full ci

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

guest-svmhdvn edited reviewers, added: lwhsu, bofh; removed: kp.

There are some inconsistencies with the proposed change around packages installed (full vs partial origin).

Why are some additional packages being installed? This part is not clear.

Some of the reasoning behind the other changes removing blocks of code is also not clear, e.g.,

if [ "${CITYPE}" = "smoke" ]; then
cat << EOF >> ${DESTDIR}/etc/rc.conf
freebsdci_type="smoke"
EOF
elif [ "${CITYPE}" = "full" ]; then
cat << EOF >> ${DESTDIR}/etc/rc.conf
freebsdci_type="full"
EOF
fi

There's some potential good stuff in the review, but... understanding the intent behind the change would be extremely helpful to determine whether or not the change is actually warranted, as well is being addressed, and finally, helping determine whether or not the approach being taken is the right one.

I've mostly listed the rationale, notes (e.g. about why some have full vs partial origin), and necessity for each change in each individual commit that is attached in the bugzilla link. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288150

As far as I know, there's no easy way to submit a patch series (like an email patch series with cover letter) on Phabricator, so I've attached it to the bugzilla.

Can you resubmit the code with -U9999? Or submit from arc diff?

I've mostly listed the rationale, notes (e.g. about why some have full vs partial origin), and necessity for each change in each individual commit that is attached in the bugzilla link. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288150

As far as I know, there's no easy way to submit a patch series (like an email patch series with cover letter) on Phabricator, so I've attached it to the bugzilla.

  1. Use git arc to create/manage diffs.
  2. Use a Phabricator diff stack to manage multiple changes:

The directions don't use git arc, but you can probably achieve the same thing using git arc just using slightly different arguments. If not, you'll have to resort to using the raw arcanist tools, e.g., arc diff create.

CC: @markj

Thanks! Abandoning this now. I've added a stack of reviews here: https://reviews.freebsd.org/D51303. I tried to use git-arc from /src/tools/tools/git which said that it would automatically link each pair of consecutive commits as a stack, but I had to end up manually editing related revisions in Phabricator web.