Add GitHub Action CI checks #93

ngie-eign · 2024-12-27T00:26:15Z

This is almost entirely based on work submitted by @kevemueller in #81 .

I took the liberty of cleaning it up in a followup commit so it would better match established build methodologies and be yamllint clean (more detail is in the second commit).

Example run: https://github.com/ngie-eign/atf/actions/runs/12520930757.

This allows repeatedly polling some command until it succeeds, to some timeout limit.

Sponsored by: The FreeBSD Foundation

`expr ${OPTIND} - 1` can be replaced with $((OPTIND - 1)) to avoid spawning a new processes. While it's unlikely to make much of a performance difference for a single execution, it happens once for ever test case so it will add up for a full testsuite run.

I have been trying to speed up the time it takes to run the testsuite for CheriBSD on QEMU RISC-V (originally 22 hours). One of the slowest tests is the pfctl test since ever single invocation of it spends ~90 seconds generating the list of available tests (even when running only one test). And this is after the change in r365708 that removes ~200 calls to atf_get_srcdir(). Looking at truss output it turns out that using atf-sh results in many tr processes being spawned to perform string substitution. The _atf_normalize() function tries to remove characters that are not valid in shell variable names by replacing . and - with _. However, most strings passed to the function don't contain those characters, so we unnecessarily fork() a new tr processes (which adds up to a significant slowdown). With this change we only call tr if the variable contains one of the illegal characters rather than unconditionally. Ideally we would do this substitution using only shell builtins, but unfortunately the string substitution syntax ${var//} is not supported by POSIX sh and I am not aware of any alternatives. Basic time measurements on CHERI-QEMU RISC-V usng `/usr/bin/time /usr/tests/sbin/pfctl_test -l > /dev/null`: Before: `90.68 real 89.99 user 0.10 sys` After: `14.50 real 14.31 user 0.10 sys` I.e. 85% of the time was being spent in these tr invocations! truss -cf before: ``` syscall seconds calls errors fork 453.330532000 2371 0 cap_enter 0.247944000 633 0 cap_ioctls_limit 0.761099000 1899 0 vfork 0.421971000 2 0 getegid 0.000342000 1 0 getgid 0.000358000 1 0 getuid 0.000324000 1 0 getppid 0.000589000 1 0 geteuid 0.000631000 2 0 getpid 0.000328000 1 0 issetugid 0.450306000 1270 0 write 0.415779000 634 0 __sysctl 0.280936000 634 0 sigprocmask 2.543827000 6340 0 readlink 0.317952000 635 635 read 1.319873000 2532 0 pread 0.327678000 634 0 open 1.399164000 2536 634 munmap 0.301268000 634 0 mprotect 0.566142000 1268 0 mmap 7.117084000 8876 0 fstat 1.326981000 3168 0 dup2 0.954023000 2373 0 close 4.496506000 9979 0 clock_gettime 0.740010000 1902 0 cap_rights_limit 0.770141000 1899 0 cap_fcntls_limit 0.695135000 1899 0 ------------- ------- ------- 478.786923000 52125 1269 ``` After: ``` syscall seconds calls errors fork 24.675445000 1107 0 cap_enter 0.000340000 1 0 cap_ioctls_limit 0.001009000 3 0 vfork 0.228345000 1 0 getegid 0.000318000 1 0 getgid 0.000323000 1 0 getuid 0.000345000 1 0 getppid 0.000329000 1 0 geteuid 0.000726000 2 0 getpid 0.000335000 1 0 issetugid 0.001554000 4 0 write 0.000539000 1 0 __sysctl 0.000369000 1 0 sigprocmask 0.003684000 10 0 readlink 0.000440000 1 1 read 0.001318000 2 0 pread 0.000556000 1 0 open 0.002250000 4 1 munmap 0.000524000 1 0 mprotect 0.000767000 2 0 mmap 0.011109000 14 0 fstat 0.001743000 4 0 dup2 0.398638000 1108 0 close 1.996684000 4916 0 clock_gettime 0.001047000 3 0 cap_rights_limit 0.001170000 3 0 cap_fcntls_limit 0.000930000 3 0 ------------- ------- ------- 27.330837000 7197 2 ```

atf-sh/atf-check.1[126]: Sentence does not start on new line

Also create a junit xml file.

CirrusCI doesn't like iso-8859-1, hopefully it accepts utf-8. The test outputs should only contain ASCII characters so this shouldn't matter.

… log

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

There are more of these that need to be modernized, but this will do for now. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

We will be reusing this when we later allow the results file to be closed and reopened for internal test reasons, so do this cleanup in advance. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

Don't bother with different logic based on ctx->resfile, just set ctx->resfilefd up initially with STDOUT_FILENO/STDERR_FILENO and write to the ctx->resfilefd unconditionally later. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

This allows us to set the resultsfile mid-run if we need to, as will be the case for some of our atf internal tests. Notably, some internal tests will fork and do ATF_REQUIRE's that we do -not- want to be written to our outer-test's results file. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

This will be needed for some internal tests that currently fail as they invoke some inner-test that's supposed to fail, but it isn't supposed to write to the outer test's result file. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

All of the tests that fork_and_wait will fork(), fork() again, print some stuff and exit, then the middle-fork parent will wait for the exit and do some checks. These checks are expected to fail in some cases, but what we aren't expecting is for them to use the same resultfile (oops!). Fix this by using our new API to reset the result file to /dev/null in the middle-fork parent, so that our meta-tests are OK. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

If any atf_expect_*() function is called, then we currently close() the resultfile after the expectation is written then proceed. If we end up *failing* that expectation, then we'll venture across this closed fd (unless we're outputting to stdout/stderr) and fail. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

The Cirrus environment may not have CC/CXX set, in which case we should use some sensible default. This causes problems as the compiled product actually tries to use ${ATF_BUILD_CC} and ${ATF_BUILD_CXX}, and this fails miserably as they're ultimately empty. However, we also don't really need to do any of this if we don't have any archflags to add, so just don't. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

The exception is stdout/stderr, which we won't even attempt in case it's been redirected to a file in a level above us that wants to check expectations or what-have-you. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

- Unconditionally bootstrap pkg to avoid pkg - Let job name match `uname -r` (13.0-CURRENT) - Chase latest 12.x-STABLE image Sponsored by: The FreeBSD Foundation

Sponsored by: The FreeBSD Foundation

The error is quite obvious and looks like copy/paste mistake.

Single quotes around commands in examples do not help anything, and are actually harmful in case of 'echo foo'.

This adds ATF_{CHECK,REQUIRE}_INTEQ to match the ATF_{CHECK,REQUIRE}_STREQ macros. This should make debugging failed test cases a lot easier.

Switch the release image from 12.1 (no longer supported by ports) to 13.0 (the latest). Also test on stable/13 and head.

I'm working on a test that will fail due to a kernel bug. However, it can't be executed unless certain hardware is present, and I can't check if the hardware supports the test without running the entire test. So it makes sense to set atf_tc_expect_fail in the beginning and later to atf_tc_skip if the hardware isn't suitable. But ATF doesn't currently allow that.

Quoted multi-word commands will be passed literally to exec rather than split on arguments, which will simply not work. Stop excessive quotes to make sure we don't mislead folks into thinking they're needed, and slap -x on the one example that needs it. This is a follow up to PR freebsd#13.

This proposed GitHub Actions workflow file runs tests on MacOS and Ubuntu Linux, similar to what is being proposed in freebsd/atf#93 . The delta between this change and the one being made in ATF mostly relates to the additional package dependencies, e.g., with and without ATF. Kyua is built from scratch because of issues with dependent packages and runtime linker collisions on both MacOS and Ubuntu. The binary package versions are built against a different version of Lutok, ergo they can have runtime linker collisions. Signed-off-by: Enji Cooper <ngie@FreeBSD.org>

* admin/make-release.sh: automate release creation This does the absolute bare minimum to produce a release. This logic best resembles the expectations for legacy (pre-0.22) releases of ATF. Various files are omitted as they are not required in order to build from a full `autoreconf`'ed source distribution. Signed-off-by: Enji Cooper <ngie@FreeBSD.org>

This proposed GitHub Actions workflow file runs tests on MacOS and Ubuntu Linux, similar to what is being proposed in freebsd/atf#93 . The delta between this change and the one being made in ATF mostly relates to the additional package dependencies, e.g., with and without ATF. Kyua is built from scratch because of issues with dependent packages and runtime linker collisions on both MacOS and Ubuntu. The binary package versions are built against a different version of Lutok, ergo they can have runtime linker collisions. Signed-off-by: Enji Cooper <ngie@FreeBSD.org>

atf::fs::path has a str() method, and it returns a std::string constructor. I found this odd that a vector is used here to copy std::string contents. As std::string is a typedef of std::basic_string<char>, this additional copy is not needed. Also, remove calling the str() method on error. Notes by ngie@: `buf.data()` returns `const char*` until C++17, so it can't be used in combination with `mkstemp(..)` (it mutates the template argument). Access the internal buffer with the `operator[]` instead. Add the C++17+ form as well along with a TODO comment, so the code can move to std::string::data eventually. Co-authored by: Enji Cooper <ngie@FreeBSD.org> Signed-off-by: rilysh <nightquick@proton.me> Co-authored-by: rilysh <nightquick@proton.me>

…figurations Build on macOS Sonoma/aarch64 and Ubuntu 24.04/amd64.

- Clean up the YAML to make it yamllint clean - Remove some references to sanitizers so the code is isolated/standalone. - Change branch from `main` to `master`, since development mainline is still `master`, not `main`. - Rename the steps for human consumption. - Change the environment variable names in the "Setup Environment" step to match a scheme that's easier to sort and to align better with established build methodologies (srcdir, objdir, prefix). - Use `installcheck` instead of `installcheck-kyua`. Signed-off-by: Enji Cooper <ngie@FreeBSD.org>

.github/workflows/build.yaml

emaste · 2024-12-29T23:49:02Z

Can we perhaps add FreeBSD support via https://github.com/vmactions/freebsd-vm (until GH Actions has native FreeBSD support)?

asomers · 2024-12-30T15:24:29Z

Can we perhaps add FreeBSD support via https://github.com/vmactions/freebsd-vm (until GH Actions has native FreeBSD support)?

It's easier to use Cirrus CI, IMHO, because it supports FreeBSD natively.

* Garbage collect stray reference to auto_array This entry should have been removed in the referenced commit. Fixes: 6639d08 * Require C++-14 at bare minimum This change imports ac_cxx_compile_stdcxx.m4 from gnu,org and makes use of the `AX_CXX_COMPILE_STDCXX` macro in configure.ac to ensure that the compiler specified supports C++-14 (at bare minimum). This is being done to quell some issues reported by scan-build about the code using C++ range-based for-loops (a feature added in C++-11). * Stop shipping atf-*-api(3) manpages These manpages have been deprecated for at least a release. Remove them and all of the logic associated with them. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * Expose WCOREDUMP(..) in a deterministic manner WCOREDUMP is considered an extension to the POSIX spec on multiple platforms, and thus is not automatically exposed on all platforms. Add the relevant preprocessor defines to config.h via autoconf, then leverage them in atf-c(3). This helps ensure that the platforms which support WCOREDUMP properly expose the macro. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * Add compiler feature detection for `__attribute__((nonnull))` This feature is being detected so several functions can be appropriately marked as taking non-NULL/-nullptr parameters, and the compiler and static analyzers can (in turn) make intelligent decisions when optimizing and analyzing code, respectively. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * Restore AC_PROG_CPP This was accidentally deleted post-0.21 release. It's still needed by tests and build infrastructure. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * atf-c/detail/env.c: remove unnecessary complexity The libcalls used have been in the POSIX standard since 2008.1. Require them to be present instead of conditionally hoping they're present. This fixes an issue where the autoconf code was messing up with a combination of clang tools, which resulted in the autoconf code failing to properly determine whether or not the functions were available. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * Remove ATF_BUILD_CXX require.progs check This particular check unfortunately doesn't work when ATF_BUILD_CXX contains multiple CXXFLAGS, or contains a path with spaces in it. Remove this check to unbreak the dependent tests post-793d4640031dc06ce8a239ffa9ab61322104c4ca. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> * Remove redundant C++ toolchain check The C++ toolchain is sanity checked when C++14 conformance is checked; there's no reason why we need to check whether or not the C++ toolchain works again. Signed-off-by: Enji Cooper <ngie@FreeBSD.org> --------- Signed-off-by: Enji Cooper <ngie@FreeBSD.org>

cemeyer and others added 30 commits November 25, 2024 14:28

Add -r timeout[:interval] option to atf-check.

b32455e

This allows repeatedly polling some command until it succeeds, to some timeout limit.

Fix freebsd#45: Preopen result files before running testcases

d792396

Add Cirrus CI config for FreeBSD build

26b91d5

Sponsored by: The FreeBSD Foundation

Fix make distcheck warning that is causing CI to fail

5015806

atf-sh/atf-check.1[126]: Sentence does not start on new line

List test results in Cirrus CI using kyua report

9d22566

Also create a junit xml file.

CirrusCI: install kyua to run tests

0c601f0

Patch the kyua junit xml encoding

99be12b

CirrusCI doesn't like iso-8859-1, hopefully it accepts utf-8. The test outputs should only contain ASCII characters so this shouldn't matter.

List available files on failure to find the correct path to the build…

d8ca354

… log

admin: correct path to the config.log

c426c0a

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

atf-c++: tests: switch pimpl to a unique_ptr; auto_ptr is deprecated

6ad526d

There are more of these that need to be modernized, but this will do for now. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

atf-c/tc: factor out closing of the results file

892c429

We will be reusing this when we later allow the results file to be closed and reopened for internal test reasons, so do this cleanup in advance. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

atf-c/tc: simplify create_resfile()

73adddd

Don't bother with different logic based on ctx->resfile, just set ctx->resfilefd up initially with STDOUT_FILENO/STDERR_FILENO and write to the ctx->resfilefd unconditionally later. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

atf-c++: also plumb through our API for atf_utils_reset_resultsfile

02018f5

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

atf-c/tc: do truncate the results file before writing a new one

d8a542e

The exception is stdout/stderr, which we won't even attempt in case it's been redirected to a file in a level above us that wants to check expectations or what-have-you. Signed-off-by: Kyle Evans <kevans@FreeBSD.org>

Few minor improvements to CI

637e1b4

- Unconditionally bootstrap pkg to avoid pkg - Let job name match `uname -r` (13.0-CURRENT) - Chase latest 12.x-STABLE image Sponsored by: The FreeBSD Foundation

Skip updating all installed packages to speed up installation step

15dbbaa

Sponsored by: The FreeBSD Foundation

atf-check: fix reporting wrong missing file name

399c559

The error is quite obvious and looks like copy/paste mistake.

atf-sh.3: don't quote commands in examples

e35d511

Single quotes around commands in examples do not help anything, and are actually harmful in case of 'echo foo'.

Add macros to compare integers with a helpful error message

616707a

This adds ATF_{CHECK,REQUIRE}_INTEQ to match the ATF_{CHECK,REQUIRE}_STREQ macros. This should make debugging failed test cases a lot easier.

Update CI testing images

e62a50f

Switch the release image from 12.1 (no longer supported by ports) to 13.0 (the latest). Also test on stable/13 and head.

Install pkgconf during CI for better test coverage

30f2026

ngie-eign mentioned this pull request Dec 29, 2024

Add initial GitHub Actions support for kyua freebsd/kyua#254

Open

ngie-eign and others added 3 commits December 29, 2024 12:12

Introduce GH action to build&tests atf with potentially different con…

52f7a6f

…figurations Build on macOS Sonoma/aarch64 and Ubuntu 24.04/amd64.

ngie-eign force-pushed the add-gh-actions branch from 8703dd0 to 1aaf58a Compare December 29, 2024 20:14

emaste reviewed Dec 29, 2024

View reviewed changes

.github/workflows/build.yaml Outdated Show resolved Hide resolved

emaste approved these changes Dec 29, 2024

View reviewed changes

fel1x-developer and others added 4 commits December 30, 2024 13:57

configure.ac: Update mailing list and repo URL

7cf34b3

Fix prototype of atf_utils_file_exists in the manual page (freebsd#82)

e4ba675

Merge branch 'master' into add-gh-actions

19ce6f6

ngie-eign force-pushed the master branch from bebd79c to 6ef7e11 Compare March 29, 2025 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GitHub Action CI checks #93

Add GitHub Action CI checks #93

ngie-eign commented Dec 27, 2024 •

edited

Loading

emaste commented Dec 29, 2024

asomers commented Dec 30, 2024

Add GitHub Action CI checks #93

Are you sure you want to change the base?

Add GitHub Action CI checks #93

Conversation

ngie-eign commented Dec 27, 2024 • edited Loading

emaste commented Dec 29, 2024

asomers commented Dec 30, 2024

ngie-eign commented Dec 27, 2024 •

edited

Loading