* [PATCH 0/3] CI: Improvements to *-tools-test-* jobs
@ 2025-05-20 20:52 Andrew Cooper
2025-05-20 20:52 ` [PATCH 1/3] tools/tests: Drop depriv-fd-checker Andrew Cooper
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Andrew Cooper @ 2025-05-20 20:52 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Anthony PERARD, Stefano Stabellini,
Marek Marczykowski-Górecki
Rearrange tools/tests to be more ameanable to running in CI, and drop the
special casing holding it together.
Andrew Cooper (3):
tools/tests: Drop depriv-fd-checker
tools/tests: Install tests into $(LIBEXEC)/tests
CI: Drop custom handling of tools/tests
.gitignore | 1 -
automation/scripts/build | 1 -
automation/scripts/qubes-x86-64.sh | 7 +-
automation/scripts/run-tools-tests | 43 ++-
tools/tests/Makefile | 1 -
tools/tests/cpu-policy/Makefile | 6 +-
tools/tests/depriv/Makefile | 52 ---
tools/tests/depriv/depriv-fd-checker.c | 436 -------------------------
tools/tests/paging-mempool/Makefile | 6 +-
tools/tests/rangeset/Makefile | 6 +-
tools/tests/resource/Makefile | 6 +-
tools/tests/tsx/Makefile | 6 +-
tools/tests/xenstore/Makefile | 6 +-
13 files changed, 40 insertions(+), 537 deletions(-)
delete mode 100644 tools/tests/depriv/Makefile
delete mode 100644 tools/tests/depriv/depriv-fd-checker.c
--
2.39.5
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] tools/tests: Drop depriv-fd-checker 2025-05-20 20:52 [PATCH 0/3] CI: Improvements to *-tools-test-* jobs Andrew Cooper @ 2025-05-20 20:52 ` Andrew Cooper 2025-05-26 16:42 ` Anthony PERARD 2025-05-20 20:52 ` [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests Andrew Cooper ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2025-05-20 20:52 UTC (permalink / raw) To: Xen-devel Cc: Andrew Cooper, Anthony PERARD, Stefano Stabellini, Marek Marczykowski-Górecki Unlike the other tests, this is not standalone. It requires poking at a live system, making it unweildly to use. It hasn't been touched in 7 years, despite changes in libraries and kernel devices using the deprivilege infrastructure. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- .gitignore | 1 - tools/tests/Makefile | 1 - tools/tests/depriv/Makefile | 52 --- tools/tests/depriv/depriv-fd-checker.c | 436 ------------------------- 4 files changed, 490 deletions(-) delete mode 100644 tools/tests/depriv/Makefile delete mode 100644 tools/tests/depriv/depriv-fd-checker.c diff --git a/.gitignore b/.gitignore index 53f5df000383..4a4e20680464 100644 --- a/.gitignore +++ b/.gitignore @@ -165,7 +165,6 @@ tools/misc/xencov tools/pkg-config/* tools/qemu-xen-build tools/xentrace/xenalyze -tools/tests/depriv/depriv-fd-checker tools/tests/x86_emulator/*.bin tools/tests/x86_emulator/*.tmp tools/tests/x86_emulator/32/x86_emulate diff --git a/tools/tests/Makefile b/tools/tests/Makefile index 3e60ab6b268d..36928676a666 100644 --- a/tools/tests/Makefile +++ b/tools/tests/Makefile @@ -9,7 +9,6 @@ ifneq ($(clang),y) SUBDIRS-$(CONFIG_X86) += x86_emulator endif SUBDIRS-y += xenstore -SUBDIRS-y += depriv SUBDIRS-y += rangeset SUBDIRS-y += vpci SUBDIRS-y += paging-mempool diff --git a/tools/tests/depriv/Makefile b/tools/tests/depriv/Makefile deleted file mode 100644 index 5404a12f4780..000000000000 --- a/tools/tests/depriv/Makefile +++ /dev/null @@ -1,52 +0,0 @@ -XEN_ROOT=$(CURDIR)/../../.. -include $(XEN_ROOT)/tools/Rules.mk - -CFLAGS += $(CFLAGS_xeninclude) -CFLAGS += $(CFLAGS_libxenctrl) -CFLAGS += $(CFLAGS_libxencall) -CFLAGS += $(CFLAGS_libxenevtchn) -CFLAGS += $(CFLAGS_libxengnttab) -CFLAGS += $(CFLAGS_libxenforeignmemory) -CFLAGS += $(CFLAGS_libxendevicemodel) -CFLAGS += $(CFLAGS_libxentoolcore) -CFLAGS += $(CFLAGS_libxentoollog) - -LDLIBS += $(LDLIBS_xeninclude) -LDLIBS += $(LDLIBS_libxenctrl) -LDLIBS += $(LDLIBS_libxencall) -LDLIBS += $(LDLIBS_libxenevtchn) -LDLIBS += $(LDLIBS_libxengnttab) -LDLIBS += $(LDLIBS_libxenforeignmemory) -LDLIBS += $(LDLIBS_libxendevicemodel) -LDLIBS += $(LDLIBS_libxentoolcore) -LDLIBS += $(LDLIBS_libxentoollog) - -INSTALL_PRIVBIN-y += depriv-fd-checker -INSTALL_PRIVBIN := $(INSTALL_PRIVBIN-y) -TARGETS += $(INSTALL_PRIVBIN) - -.PHONY: all -all: build - -.PHONY: build -build: $(TARGETS) - -.PHONY: clean -clean: - $(RM) *.o $(TARGETS) *~ $(DEPS_RM) - -.PHONY: distclean -distclean: clean - -depriv-fd-checker: depriv-fd-checker.o - $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS) $(APPEND_LDFLAGS) - -install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(INSTALL_PROG) $(INSTALL_PRIVBIN) $(DESTDIR)$(LIBEXEC_BIN) - -.PHONY: uninstall -uninstall: - rm -f $(addprefix $(DESTDIR)$(LIBEXEC_BIN)/, $(INSTALL_PRIVBIN)) - --include $(DEPS_INCLUDE) diff --git a/tools/tests/depriv/depriv-fd-checker.c b/tools/tests/depriv/depriv-fd-checker.c deleted file mode 100644 index 98a27a03d543..000000000000 --- a/tools/tests/depriv/depriv-fd-checker.c +++ /dev/null @@ -1,436 +0,0 @@ -/* - * depriv-fd-checker - * - * utility to check whether file descriptor(s) are deprivileged - * - * usage: - * .../depriv-fd-checker CLASS FD X-INFO [CLASS FD X-INFO...] - * - * CLASS is one of: - * privcmd gntdev evtchn FD should be appropriate Xen control fd - * readonly FD is expected to be readonly - * appendonly FD is expected to be append write only - # tun FD is expected to be an open tun device - * - * In each case FD is probably a reference to an open-file stolen - * from another process, eg by the use of fishdescriptor. - * - * X-INFO is simply appended to the discursive reportage. - * - * It is an error if depriv-fd-checker cannot open the control - * facilities itself, or something goes wrong with checking, or an FD - * is entirely the wrong kind for the specified CLASS. Otherwise: - * - * depriv-fd-checker will perhaps print, for each triplet: - * CLASS checking FD INFORMATION... X-INFO - * and in any case print, for each triplet, exactly one of: - * CLASS pass|fail FD INFORMATION... X-INFO - * tun maybe FD IFNAME X-INFO - * - * "pass" means that the descriptor was restricted as expected. - * "fail" means that the descriptor was unrestricted. - * "maybe" means that further information is printed, as detailed above, - * and the caller should check that it is as expected - */ -/* - * Copyright (C)2018 Citrix Systems R&D - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public License - * as published by the Free Software Foundation; version 2.1 of the - * License. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; If not, see - * <http://www.gnu.org/licenses/>. - */ - -#include <stdlib.h> -#include <errno.h> -#include <string.h> -#include <stdio.h> -#include <assert.h> -#include <string.h> -#include <unistd.h> -#include <fcntl.h> -#include <poll.h> - -#include <err.h> - -#include <xenctrl.h> -#include <xencall.h> -#include <xengnttab.h> -#include <xenevtchn.h> - -/* - * Every class needs setup. setup is called once per class at program - * startup. - * - * Then it can have - * open test getfd close - * In which case the core code will for every fd - * open test getfd dup2 test close - * And test should call blocked or succeeded and then immediately - * return, or error out - * - * Or it can have - * check - * which should call report, or error out - * - * Errors: use trouble for simple syscall errors. Or use err or errx - * and maybe print fd_desc and test_which, according to the comments - * in struct classinfo. - */ - -static xentoollog_logger *logger; - -static int object_fd; -static const char *classname; -static const char *fd_desc; -static const char *test_which; - -static const char *test_wh_unrest = "test (unrestricted)"; -static const char *test_wh_rest = "test (restricted)"; - - -static void trouble(const char *what) __attribute__((noreturn)); -static void trouble(const char *what) { - fprintf(stderr, - "trouble: %s %s %d (%s) %s: %s\n", - classname, test_which, object_fd, fd_desc, what, strerror(errno)); - exit(-1); -} - -static void report(const char *pass_or_fail, const char *what, - const char *notes) { - printf("%s %s %d %s (%s) %s\n", - classname, pass_or_fail, - object_fd, what, notes, fd_desc); - if (ferror(stdout) || fflush(stdout)) err(16,"stdout"); -} - -static void succeeded(const char *what) { - if (test_which == test_wh_unrest) { - /* ok */ - test_which = 0; - } else if (test_which == test_wh_rest) { - report("fail",what,"unexpectedly succeeded"); - test_which = 0; - } else { - abort(); - } -} - -static void blocked(const char *what) { - if (test_which == test_wh_rest) { - /* yay */ - report("pass", what,"blocked"); - test_which = 0; - } else if (test_which == test_wh_unrest) { - err(4,"test blocked on unrestricted fd: %s {%s}",what,test_which); - } else { - abort(); - } -} - -/* privcmd */ - -static xc_interface *xch; -static void setup_privcmd(void) { } -static void open_privcmd(void) { - xch = xc_interface_open(logger,0,0); - if (!xch) trouble("xc_interface_open"); -} -static void test_privcmd(void) { - int r = xc_get_online_cpus(xch); - if (r>0) - succeeded("xc_get_online_cpus"); - else if (r==0) - errx(-1,"xc_get_online_cpus{%s, %s}=0", test_which, fd_desc); - else if (errno==EPERM || errno==EACCES) - blocked("xc_get_online_cpus"); - else - trouble("xc_get_online_cpus"); -} -static int getfd_privcmd(void) { - return xencall_fd(xc_interface_xcall_handle(xch)); -} -static void close_privcmd(void) { - xc_interface_close(xch); -} - -/* gntdev */ - -static xengntshr_handle *xgs; -static uint32_t gntshr_gref; -static xengnttab_handle *xgt; -static void setup_gntdev(void) { - void *r; - xgs = xengntshr_open(logger,0); - if (!xgs) trouble("xengntshr_open"); - r = xengntshr_share_pages(xgs, 0, 1, &gntshr_gref, 1); - if (!r || r==(void*)-1) trouble("xengntshr_share_pages"); - memset(r, 0x55, XC_PAGE_SIZE); -} -static void open_gntdev(void) { - xgt = xengnttab_open(logger,0); - if (!xgt) trouble("xengnttab_open"); -} -static void test_gntdev(void) { - char mybuf[XC_PAGE_SIZE]; - memset(mybuf, 0xaa, XC_PAGE_SIZE); - xengnttab_grant_copy_segment_t seg; - seg.source.foreign.ref = gntshr_gref; - seg.source.foreign.offset = 0; - seg.source.foreign.domid = 0; - seg.dest.virt = mybuf; - seg.len = 1; - seg.flags = GNTCOPY_source_gref; - for (;;) { - seg.status = 0; - int r = xengnttab_grant_copy(xgt,1,&seg); - if (r<0) { - if (errno==EPERM || errno==EACCES || errno==ENOTTY) - blocked("xengnttab_grant_copy"); - else - trouble("xengnttab_grant_copy"); - } else if (r==0) { - if (seg.status==GNTST_okay) - succeeded("xengnttab_grant_copy okay"); - else if (seg.status==GNTST_eagain) - continue; - else errx(-1,"xengnttab_grant_copy=%d {%s, %s} but .status=%d", - r, test_which, fd_desc,(int)seg.status); - } else { - errx(-1,"xengnttab_grant_copy=%d {%s, %s}", - r, test_which, fd_desc); - } - break; - } -} -static int getfd_gntdev(void) { - return xengnttab_fd(xgt); -} -static void close_gntdev(void) { - xengnttab_close(xgt); -} - -/* evtchn */ - -static xenevtchn_handle *xce_recip, *xce; -static void setup_evtchn(void) { - xce_recip = xenevtchn_open(logger, 0); - if (!xce_recip) err(-1,"xenevtchn_open (donor)"); -} -static void open_evtchn(void) { - xce = xenevtchn_open(logger, 0); - if (!xce) err(-1,"xenevtchn_open"); -} -static void test_evtchn(void) { - xenevtchn_port_or_error_t - recip_port=-1, test_unbound_port=-1, test_send_port=-1; - - recip_port = xenevtchn_bind_unbound_port(xce_recip, 0); - if (recip_port < 0) trouble("xenevtchn_bind_unbound_port"); - - test_unbound_port = xenevtchn_bind_unbound_port(xce, 0); - if (test_unbound_port >= 0) { - succeeded("xenevtchn_bind_unbound_port"); - goto out; - } - - test_send_port = xenevtchn_bind_interdomain(xce, 0, recip_port); - /* bind_interdomain marks the channel pending */ - struct pollfd pfd; - for (;;) { - pfd.fd = xenevtchn_fd(xce_recip); - pfd.events = POLLIN; - pfd.revents = 0; - int r = poll(&pfd,1,0); - if (r>=0) break; - if (errno!=EINTR) err(-1,"poll(xce_recip)"); - } - if (pfd.revents & POLLIN) { - xenevtchn_port_or_error_t p3 = xenevtchn_pending(xce_recip); - if (p3 < 0) err(-1,"xenevtchn_pending(check)"); - if (p3 != recip_port) - errx(-1,"xenevtchn_pending=%d expected %d",p3,recip_port); - xenevtchn_unmask(xce_recip, recip_port); - } - - if (test_send_port>=0 && (pfd.revents & POLLIN)) { - succeeded("xenevtchn_bind_interdomain/poll"); - /* we make no attempt to undo what we did to this stolen fd; - * the rightful owner will see a spurious event on test_send_port */ - } else if (test_send_port==-1 && !(pfd.revents & POLLIN) && - (errno==EPERM || errno==EACCES || errno==ENOTTY)) { - blocked("xenevtchn_notify"); - } else { - err(-1,"%s %s xenevtchn_bind_interdomain=%d .revents=0x%x", - test_which, fd_desc, test_send_port, pfd.revents); - } - - out: - if (recip_port > 0) xenevtchn_unbind(xce, recip_port); - if (test_unbound_port > 0) xenevtchn_unbind(xce, test_unbound_port); - if (test_send_port > 0) xenevtchn_unbind(xce, test_send_port); -} -static int getfd_evtchn(void) { - return xenevtchn_fd(xce); -} -static void close_evtchn(void) { - xenevtchn_close(xce); -} - -/* fcntl */ - -#define CHECK_FCNTL(openmode) \ - int r = fcntl(object_fd, F_GETFL); \ - if (r < 0) trouble("fcntl F_GETFL"); \ - int m = r & (O_RDONLY | O_WRONLY | O_RDWR); \ - \ - char mbuf[100 + 30*3]; \ - snprintf(mbuf,sizeof(mbuf), \ - "F_GETFL=%#o m=%#o " #openmode "=%#o", \ - r,m,(int)openmode); \ - \ - if (m != openmode) { \ - report("fail", #openmode, mbuf); \ - return; \ - } - -/* readonly */ - -static void setup_readonly(void) { } -static void check_readonly(void) { - CHECK_FCNTL(O_RDONLY); - report("pass", "fcntl", mbuf); -} - -/* appendonly */ - -static void setup_appendonly(void) { } -static void check_appendonly(void) { - CHECK_FCNTL(O_WRONLY); - if (!(r & O_APPEND)) { - report("fail", "O_APPEND", mbuf); - return; - } - report("pass", "fcntl", mbuf); -} - -#if defined(__linux__) -#include <sys/ioctl.h> -#include <sys/types.h> -#include <sys/socket.h> -#include <linux/if.h> -#include <linux/if_tun.h> -#ifndef TUNGETIFF -#define TUNGETIFF _IOR('T', 210, unsigned int) -#endif - -/* linux tun */ - -static void setup_tun(void) { } -static void check_tun(void) { - struct ifreq ifr; - int r; - - memset(&ifr,0,sizeof(ifr)); - r = ioctl(object_fd, TUNGETIFF, (void*)&ifr); - if (r<0) trouble("TUNGETIFF"); - printf("tun maybe %d %.*s %s\n", object_fd, - (int)IFNAMSIZ, ifr.ifr_ifrn.ifrn_name, - fd_desc); -} - -#define PLATFORM_CLASSES \ - DEFCHECK(tun), - -#else /* !defined(__linux__) */ -#define PLATFORM_CLASSES /* empty */ -#endif - -/* class table and main program */ - -#define DEFCLASS(cl) \ - { #cl, setup_##cl, 0, open_##cl, test_##cl, getfd_##cl, close_##cl } -#define DEFCHECK(meth) \ - { #meth, setup_##meth, check_##meth } - -static const struct classinfo { - const char *name; /* errors: print fd_desc test_which */ - void (*setup)(void); /* best not best not */ - void (*check)(void); /* must may */ - void (*open)(void); /* must may */ - void (*test)(void); /* must must */ - int (*getfd)(void); /* must may */ - void (*close)(void); /* must may */ -} classinfos[] = { - DEFCLASS(privcmd), - DEFCLASS(gntdev), - DEFCLASS(evtchn), - DEFCHECK(readonly), - DEFCHECK(appendonly), - PLATFORM_CLASSES - { 0 } -}; - -int main(int argc, char **argv) { - const struct classinfo *cli; - int r; - - argv++; - - logger = (xentoollog_logger*)xtl_createlogger_stdiostream - (stderr, XTL_NOTICE, XTL_STDIOSTREAM_HIDE_PROGRESS); - - fd_desc = "setup"; - test_which = "setup"; - for (cli = classinfos; cli->name; cli++) - cli->setup(); - - while ((classname = *argv++)) { - if (!*argv) errx(8,"need fd after class"); - object_fd = atoi(*argv++); - - fd_desc = *argv++; - if (!fd_desc) errx(8,"need info after fd"); - - for (cli = classinfos; cli->name; cli++) - if (!strcmp(cli->name, classname)) - goto found; - report("fail","unknown class",""); - continue; - - found: - if (cli->check) { - report("checking","check","in progress"); - test_which = "check"; - cli->check(); - } else { - test_which = "open"; - report("checking","dup-hack","in progress"); - cli->open(); - - test_which = test_wh_unrest; cli->test(); - assert(!test_which); - - test_which = "getfd"; int intern_fd = cli->getfd(); - r = dup2(object_fd, intern_fd); - if (r != intern_fd) err(-1, "dup2"); - - test_which = test_wh_rest; cli->test(); - assert(!test_which); - - test_which = "close"; cli->close(); - } - } - - return 0; -} -- 2.39.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools/tests: Drop depriv-fd-checker 2025-05-20 20:52 ` [PATCH 1/3] tools/tests: Drop depriv-fd-checker Andrew Cooper @ 2025-05-26 16:42 ` Anthony PERARD 2025-05-28 0:07 ` Stefano Stabellini 0 siblings, 1 reply; 14+ messages in thread From: Anthony PERARD @ 2025-05-26 16:42 UTC (permalink / raw) To: Andrew Cooper Cc: Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On Tue, May 20, 2025 at 09:52:37PM +0100, Andrew Cooper wrote: > Unlike the other tests, this is not standalone. It requires poking at a live > system, making it unweildly to use. > > It hasn't been touched in 7 years, despite changes in libraries and kernel > devices using the deprivilege infrastructure. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools/tests: Drop depriv-fd-checker 2025-05-26 16:42 ` Anthony PERARD @ 2025-05-28 0:07 ` Stefano Stabellini 0 siblings, 0 replies; 14+ messages in thread From: Stefano Stabellini @ 2025-05-28 0:07 UTC (permalink / raw) To: Anthony PERARD Cc: Andrew Cooper, Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On Mon, 26 May 2025, Anthony PERARD wrote: > On Tue, May 20, 2025 at 09:52:37PM +0100, Andrew Cooper wrote: > > Unlike the other tests, this is not standalone. It requires poking at a live > > system, making it unweildly to use. > > > > It hasn't been touched in 7 years, despite changes in libraries and kernel > > devices using the deprivilege infrastructure. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Acked-by: Anthony PERARD <anthony.perard@vates.tech> Acked-by: Stefano Stabellini <sstabellini@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests 2025-05-20 20:52 [PATCH 0/3] CI: Improvements to *-tools-test-* jobs Andrew Cooper 2025-05-20 20:52 ` [PATCH 1/3] tools/tests: Drop depriv-fd-checker Andrew Cooper @ 2025-05-20 20:52 ` Andrew Cooper 2025-05-26 16:56 ` Anthony PERARD 2025-05-20 20:52 ` [PATCH 3/3] CI: Drop custom handling of tools/tests Andrew Cooper 2025-05-21 0:10 ` [PATCH 0/3] CI: Improvements to *-tools-test-* jobs dmkhn 3 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2025-05-20 20:52 UTC (permalink / raw) To: Xen-devel Cc: Andrew Cooper, Anthony PERARD, Stefano Stabellini, Marek Marczykowski-Górecki $(LIBEXEC_BIN) is a dumping ground of many things. Separate the "clearly tests" from everything else so we can clean up how they're run in CI. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- tools/tests/cpu-policy/Makefile | 6 +++--- tools/tests/paging-mempool/Makefile | 6 +++--- tools/tests/rangeset/Makefile | 6 +++--- tools/tests/resource/Makefile | 6 +++--- tools/tests/tsx/Makefile | 6 +++--- tools/tests/xenstore/Makefile | 6 +++--- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/tests/cpu-policy/Makefile b/tools/tests/cpu-policy/Makefile index 5df9b1ebbd7e..24f87e2eca2a 100644 --- a/tools/tests/cpu-policy/Makefile +++ b/tools/tests/cpu-policy/Makefile @@ -29,12 +29,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(if $(TARGETS),$(INSTALL_PROG) $(TARGETS) $(DESTDIR)$(LIBEXEC_BIN)) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(if $(TARGETS),$(INSTALL_PROG) $(TARGETS) $(DESTDIR)$(LIBEXEC)/tests) .PHONY: uninstall uninstall: - $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC_BIN)/,$(TARGETS)) + $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC)/tests/,$(TARGETS)) CFLAGS += -D__XEN_TOOLS__ CFLAGS += $(CFLAGS_xeninclude) diff --git a/tools/tests/paging-mempool/Makefile b/tools/tests/paging-mempool/Makefile index a081b3baa514..a1e12584ce80 100644 --- a/tools/tests/paging-mempool/Makefile +++ b/tools/tests/paging-mempool/Makefile @@ -16,12 +16,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC)/tests .PHONY: uninstall uninstall: - $(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET) + $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/$(TARGET) CFLAGS += $(CFLAGS_xeninclude) CFLAGS += $(CFLAGS_libxenctrl) diff --git a/tools/tests/rangeset/Makefile b/tools/tests/rangeset/Makefile index 3dafcbd0546c..e3bfce471cd3 100644 --- a/tools/tests/rangeset/Makefile +++ b/tools/tests/rangeset/Makefile @@ -20,12 +20,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC)/tests .PHONY: uninstall uninstall: - $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC_BIN)/,$(TARGET)) + $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC)/tests/,$(TARGET)) list.h: $(XEN_ROOT)/xen/include/xen/list.h rangeset.h: $(XEN_ROOT)/xen/include/xen/rangeset.h diff --git a/tools/tests/resource/Makefile b/tools/tests/resource/Makefile index a5856bf09590..09d678fffe3e 100644 --- a/tools/tests/resource/Makefile +++ b/tools/tests/resource/Makefile @@ -20,12 +20,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC)/tests .PHONY: uninstall uninstall: - $(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET) + $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/$(TARGET) CFLAGS += $(CFLAGS_xeninclude) CFLAGS += $(CFLAGS_libxenctrl) diff --git a/tools/tests/tsx/Makefile b/tools/tests/tsx/Makefile index a4f516b72597..0bb7e7010347 100644 --- a/tools/tests/tsx/Makefile +++ b/tools/tests/tsx/Makefile @@ -16,12 +16,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC)/tests .PHONY: uninstall uninstall: - $(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET) + $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/$(TARGET) .PHONY: uninstall uninstall: diff --git a/tools/tests/xenstore/Makefile b/tools/tests/xenstore/Makefile index 202dda0d3c23..2ee4a1327e75 100644 --- a/tools/tests/xenstore/Makefile +++ b/tools/tests/xenstore/Makefile @@ -20,12 +20,12 @@ distclean: clean .PHONY: install install: all - $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) - $(if $(TARGETS),$(INSTALL_PROG) $(TARGETS) $(DESTDIR)$(LIBEXEC_BIN)) + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests + $(if $(TARGETS),$(INSTALL_PROG) $(TARGETS) $(DESTDIR)$(LIBEXEC)/tests) .PHONY: uninstall uninstall: - $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC_BIN)/,$(TARGETS)) + $(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC)/tests/,$(TARGETS)) CFLAGS += $(CFLAGS_libxenstore) CFLAGS += $(APPEND_CFLAGS) -- 2.39.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests 2025-05-20 20:52 ` [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests Andrew Cooper @ 2025-05-26 16:56 ` Anthony PERARD 2025-05-28 0:07 ` Stefano Stabellini 0 siblings, 1 reply; 14+ messages in thread From: Anthony PERARD @ 2025-05-26 16:56 UTC (permalink / raw) To: Andrew Cooper Cc: Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On Tue, May 20, 2025 at 09:52:38PM +0100, Andrew Cooper wrote: > $(LIBEXEC_BIN) is a dumping ground of many things. Separate the "clearly > tests" from everything else so we can clean up how they're run in CI. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests 2025-05-26 16:56 ` Anthony PERARD @ 2025-05-28 0:07 ` Stefano Stabellini 0 siblings, 0 replies; 14+ messages in thread From: Stefano Stabellini @ 2025-05-28 0:07 UTC (permalink / raw) To: Anthony PERARD Cc: Andrew Cooper, Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On Mon, 26 May 2025, Anthony PERARD wrote: > On Tue, May 20, 2025 at 09:52:38PM +0100, Andrew Cooper wrote: > > $(LIBEXEC_BIN) is a dumping ground of many things. Separate the "clearly > > tests" from everything else so we can clean up how they're run in CI. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Acked-by: Anthony PERARD <anthony.perard@vates.tech> Acked-by: Stefano Stabellini <sstabellini@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-20 20:52 [PATCH 0/3] CI: Improvements to *-tools-test-* jobs Andrew Cooper 2025-05-20 20:52 ` [PATCH 1/3] tools/tests: Drop depriv-fd-checker Andrew Cooper 2025-05-20 20:52 ` [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests Andrew Cooper @ 2025-05-20 20:52 ` Andrew Cooper 2025-05-26 16:45 ` Andrew Cooper 2025-05-21 0:10 ` [PATCH 0/3] CI: Improvements to *-tools-test-* jobs dmkhn 3 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2025-05-20 20:52 UTC (permalink / raw) To: Xen-devel Cc: Andrew Cooper, Anthony PERARD, Stefano Stabellini, Marek Marczykowski-Górecki ... and use them from their installed location. The full recusive copy of tools/tests brings in all build and intermediate artefacts. e.g. for test-tsx alone: ./tests/tsx ./tests/tsx/.test-tsx.o.d ./tests/tsx/test-tsx.o ./tests/tsx/.gitignore ./tests/tsx/test-tsx ./tests/tsx/Makefile ./tests/tsx/test-tsx.c duplicating the test binary which is also in ./usr/lib/xen/tests Rewrite run-tools-tests to run tests from their installed location (/usr/lib/xen/tests in alpine), which effectively removes the outer loop over $dir. No practical change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> This doesn't change any tests that run, although in the XML we get two fewer skips. Both skips can be fixed by giving vpci and x86_emulator some install targets --- automation/scripts/build | 1 - automation/scripts/qubes-x86-64.sh | 7 +++-- automation/scripts/run-tools-tests | 43 +++++++++++++----------------- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/automation/scripts/build b/automation/scripts/build index cdb8cd7c722b..0e7494ff6d87 100755 --- a/automation/scripts/build +++ b/automation/scripts/build @@ -109,6 +109,5 @@ else # even though dist/ contains everything, while some containers don't even # build Xen (cd dist/install; find | cpio -o -H newc | gzip) > binaries/xen-tools.cpio.gz - cp -r tools/tests binaries/ collect_xen_artefacts fi diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh index aa47ba6bf5c0..577a00238a75 100755 --- a/automation/scripts/qubes-x86-64.sh +++ b/automation/scripts/qubes-x86-64.sh @@ -136,7 +136,7 @@ done passed="test passed" domU_check="" dom0_check=" -/tests/run-tools-tests /tests /tmp/tests-junit.xml && echo \"${passed}\" +/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml && echo \"${passed}\" nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null & " if [ "${test_variant}" = "tools-tests-pvh" ]; then @@ -195,9 +195,8 @@ cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz # test-local configuration mkdir -p rootfs cd rootfs -mkdir -p boot etc/local.d -cp -ar ../binaries/tests . -cp -a ../automation/scripts/run-tools-tests tests/ +mkdir -p boot etc/local.d root +cp -a ../automation/scripts/run-tools-tests root/ echo "#!/bin/bash diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests index 770e97c3e943..8d7aa8fa5140 100755 --- a/automation/scripts/run-tools-tests +++ b/automation/scripts/run-tools-tests @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" printf '<testsuites name="tools.tests">\n' >> "$xml_out" printf ' <testsuite name="tools.tests">\n' >> "$xml_out" failed= -for dir in "$1"/*; do - [ -d "$dir" ] || continue - echo "Running test in $dir" - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" - ret= - for f in "$dir"/*; do - [ -f "$f" ] || continue - [ -x "$f" ] || continue - "$f" 2>&1 | tee /tmp/out - ret=$? - if [ "$ret" -ne 0 ]; then - echo "FAILED: $ret" - failed+=" $dir" - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" - # TODO: could use xml escaping... but current tests seems to - # produce sane output - cat /tmp/out >> "$xml_out" - printf ' </failure>\n' >> "$xml_out" - else - echo "PASSED" - fi - done - if [ -z "$ret" ]; then - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" +for f in "$1"/*; do + if [ -x "$f" ]; then + echo "SKIP: $f not executable" + continue + fi + echo "Running $f" + printf ' <testcase name="%s">\n' "$f" >> "$xml_out" + "$f" 2>&1 | tee /tmp/out + ret=$? + if [ "$ret" -ne 0 ]; then + echo "FAILED: $f" + failed+=" $f" + printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" + # TODO: could use xml escaping... but current tests seems to + # produce sane output + cat /tmp/out >> "$xml_out" + printf ' </failure>\n' >> "$xml_out" + else + echo "PASSED" fi printf ' </testcase>\n' >> "$xml_out" done -- 2.39.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-20 20:52 ` [PATCH 3/3] CI: Drop custom handling of tools/tests Andrew Cooper @ 2025-05-26 16:45 ` Andrew Cooper 2025-05-26 17:22 ` Anthony PERARD 0 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2025-05-26 16:45 UTC (permalink / raw) To: Xen-devel Cc: Anthony PERARD, Stefano Stabellini, Marek Marczykowski-Górecki On 20/05/2025 9:52 pm, Andrew Cooper wrote: > diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests > index 770e97c3e943..8d7aa8fa5140 100755 > --- a/automation/scripts/run-tools-tests > +++ b/automation/scripts/run-tools-tests > @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" > printf '<testsuites name="tools.tests">\n' >> "$xml_out" > printf ' <testsuite name="tools.tests">\n' >> "$xml_out" > failed= > -for dir in "$1"/*; do > - [ -d "$dir" ] || continue > - echo "Running test in $dir" > - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" > - ret= > - for f in "$dir"/*; do > - [ -f "$f" ] || continue > - [ -x "$f" ] || continue > - "$f" 2>&1 | tee /tmp/out > - ret=$? > - if [ "$ret" -ne 0 ]; then > - echo "FAILED: $ret" > - failed+=" $dir" > - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" > - # TODO: could use xml escaping... but current tests seems to > - # produce sane output > - cat /tmp/out >> "$xml_out" > - printf ' </failure>\n' >> "$xml_out" > - else > - echo "PASSED" > - fi > - done > - if [ -z "$ret" ]; then > - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" > +for f in "$1"/*; do > + if [ -x "$f" ]; then > + echo "SKIP: $f not executable" > + continue This should be ! -x I had that hunk in the wrong patch when posting this series. ~Andrew > + fi > + echo "Running $f" > + printf ' <testcase name="%s">\n' "$f" >> "$xml_out" > + "$f" 2>&1 | tee /tmp/out > + ret=$? > + if [ "$ret" -ne 0 ]; then > + echo "FAILED: $f" > + failed+=" $f" > + printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" > + # TODO: could use xml escaping... but current tests seems to > + # produce sane output > + cat /tmp/out >> "$xml_out" > + printf ' </failure>\n' >> "$xml_out" > + else > + echo "PASSED" > fi > printf ' </testcase>\n' >> "$xml_out" > done ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-26 16:45 ` Andrew Cooper @ 2025-05-26 17:22 ` Anthony PERARD 2025-05-26 17:31 ` Andrew Cooper 0 siblings, 1 reply; 14+ messages in thread From: Anthony PERARD @ 2025-05-26 17:22 UTC (permalink / raw) To: Andrew Cooper Cc: Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On Mon, May 26, 2025 at 05:45:29PM +0100, Andrew Cooper wrote: > On 20/05/2025 9:52 pm, Andrew Cooper wrote: > > diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests > > index 770e97c3e943..8d7aa8fa5140 100755 > > --- a/automation/scripts/run-tools-tests > > +++ b/automation/scripts/run-tools-tests > > @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" > > printf '<testsuites name="tools.tests">\n' >> "$xml_out" > > printf ' <testsuite name="tools.tests">\n' >> "$xml_out" > > failed= > > -for dir in "$1"/*; do > > - [ -d "$dir" ] || continue > > - echo "Running test in $dir" > > - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" > > - ret= > > - for f in "$dir"/*; do > > - [ -f "$f" ] || continue > > - [ -x "$f" ] || continue > > - "$f" 2>&1 | tee /tmp/out > > - ret=$? > > - if [ "$ret" -ne 0 ]; then > > - echo "FAILED: $ret" > > - failed+=" $dir" > > - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" > > - # TODO: could use xml escaping... but current tests seems to > > - # produce sane output > > - cat /tmp/out >> "$xml_out" > > - printf ' </failure>\n' >> "$xml_out" > > - else > > - echo "PASSED" > > - fi > > - done > > - if [ -z "$ret" ]; then > > - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" > > +for f in "$1"/*; do > > + if [ -x "$f" ]; then > > + echo "SKIP: $f not executable" > > + continue > > This should be ! -x > > I had that hunk in the wrong patch when posting this series. With that fixed: Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> But I think there's an issue with the script... > > + "$f" 2>&1 | tee /tmp/out > > + ret=$? > > + if [ "$ret" -ne 0 ]; then Is this checking the correct exit value? It seems that without `set -o pipefail`, we only have the exit value of `tee` which should never fail. But I think we should grab the value of ${PIPESTATUS[0]} to actually read the exit value of $f. Thanks, -- Anthony PERARD ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-26 17:22 ` Anthony PERARD @ 2025-05-26 17:31 ` Andrew Cooper 2025-05-28 0:16 ` Stefano Stabellini 0 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2025-05-26 17:31 UTC (permalink / raw) To: Anthony PERARD Cc: Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki On 26/05/2025 6:22 pm, Anthony PERARD wrote: > On Mon, May 26, 2025 at 05:45:29PM +0100, Andrew Cooper wrote: >> On 20/05/2025 9:52 pm, Andrew Cooper wrote: >>> diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests >>> index 770e97c3e943..8d7aa8fa5140 100755 >>> --- a/automation/scripts/run-tools-tests >>> +++ b/automation/scripts/run-tools-tests >>> @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" >>> printf '<testsuites name="tools.tests">\n' >> "$xml_out" >>> printf ' <testsuite name="tools.tests">\n' >> "$xml_out" >>> failed= >>> -for dir in "$1"/*; do >>> - [ -d "$dir" ] || continue >>> - echo "Running test in $dir" >>> - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" >>> - ret= >>> - for f in "$dir"/*; do >>> - [ -f "$f" ] || continue >>> - [ -x "$f" ] || continue >>> - "$f" 2>&1 | tee /tmp/out >>> - ret=$? >>> - if [ "$ret" -ne 0 ]; then >>> - echo "FAILED: $ret" >>> - failed+=" $dir" >>> - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" >>> - # TODO: could use xml escaping... but current tests seems to >>> - # produce sane output >>> - cat /tmp/out >> "$xml_out" >>> - printf ' </failure>\n' >> "$xml_out" >>> - else >>> - echo "PASSED" >>> - fi >>> - done >>> - if [ -z "$ret" ]; then >>> - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" >>> +for f in "$1"/*; do >>> + if [ -x "$f" ]; then >>> + echo "SKIP: $f not executable" >>> + continue >> This should be ! -x >> >> I had that hunk in the wrong patch when posting this series. > With that fixed: > Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks. > > But I think there's an issue with the script... > >>> + "$f" 2>&1 | tee /tmp/out >>> + ret=$? >>> + if [ "$ret" -ne 0 ]; then > Is this checking the correct exit value? It seems that without `set -o > pipefail`, we only have the exit value of `tee` which should never fail. > But I think we should grab the value of ${PIPESTATUS[0]} to actually > read the exit value of $f. Hmm yes, I think this needs adjusting. It turns out there are multiple problems with junit, including the fact that putting failures in here doesn't cause the outer job to fail. The internet suggest having a 'script: grep "<failure" junit.xml' step to work around this. I think that wants to be a separate series. The question is whether to do this series first or second. I expect I'm going to need to backport all of this work to eventually get XTF back onto the older trees. ~Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-26 17:31 ` Andrew Cooper @ 2025-05-28 0:16 ` Stefano Stabellini 2025-05-28 0:18 ` Andrew Cooper 0 siblings, 1 reply; 14+ messages in thread From: Stefano Stabellini @ 2025-05-28 0:16 UTC (permalink / raw) To: Andrew Cooper Cc: Anthony PERARD, Xen-devel, Stefano Stabellini, Marek Marczykowski-Górecki [-- Attachment #1: Type: text/plain, Size: 3058 bytes --] On Mon, 26 May 2025, Andrew Cooper wrote: > On 26/05/2025 6:22 pm, Anthony PERARD wrote: > > On Mon, May 26, 2025 at 05:45:29PM +0100, Andrew Cooper wrote: > >> On 20/05/2025 9:52 pm, Andrew Cooper wrote: > >>> diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests > >>> index 770e97c3e943..8d7aa8fa5140 100755 > >>> --- a/automation/scripts/run-tools-tests > >>> +++ b/automation/scripts/run-tools-tests > >>> @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" > >>> printf '<testsuites name="tools.tests">\n' >> "$xml_out" > >>> printf ' <testsuite name="tools.tests">\n' >> "$xml_out" > >>> failed= > >>> -for dir in "$1"/*; do > >>> - [ -d "$dir" ] || continue > >>> - echo "Running test in $dir" > >>> - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" > >>> - ret= > >>> - for f in "$dir"/*; do > >>> - [ -f "$f" ] || continue > >>> - [ -x "$f" ] || continue > >>> - "$f" 2>&1 | tee /tmp/out > >>> - ret=$? > >>> - if [ "$ret" -ne 0 ]; then > >>> - echo "FAILED: $ret" > >>> - failed+=" $dir" > >>> - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" > >>> - # TODO: could use xml escaping... but current tests seems to > >>> - # produce sane output > >>> - cat /tmp/out >> "$xml_out" > >>> - printf ' </failure>\n' >> "$xml_out" > >>> - else > >>> - echo "PASSED" > >>> - fi > >>> - done > >>> - if [ -z "$ret" ]; then > >>> - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" > >>> +for f in "$1"/*; do > >>> + if [ -x "$f" ]; then > >>> + echo "SKIP: $f not executable" > >>> + continue > >> This should be ! -x > >> > >> I had that hunk in the wrong patch when posting this series. > > With that fixed: > > Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > Thanks. > > > > > But I think there's an issue with the script... > > > >>> + "$f" 2>&1 | tee /tmp/out > >>> + ret=$? > >>> + if [ "$ret" -ne 0 ]; then > > Is this checking the correct exit value? It seems that without `set -o > > pipefail`, we only have the exit value of `tee` which should never fail. > > But I think we should grab the value of ${PIPESTATUS[0]} to actually > > read the exit value of $f. > > Hmm yes, I think this needs adjusting. > > It turns out there are multiple problems with junit, including the fact > that putting failures in here doesn't cause the outer job to fail. > > The internet suggest having a 'script: grep "<failure" junit.xml' step > to work around this. > > I think that wants to be a separate series. The question is whether to > do this series first or second. I expect I'm going to need to backport > all of this work to eventually get XTF back onto the older trees. I'll leave the choice to you ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] CI: Drop custom handling of tools/tests 2025-05-28 0:16 ` Stefano Stabellini @ 2025-05-28 0:18 ` Andrew Cooper 0 siblings, 0 replies; 14+ messages in thread From: Andrew Cooper @ 2025-05-28 0:18 UTC (permalink / raw) To: Stefano Stabellini Cc: Anthony PERARD, Xen-devel, Marek Marczykowski-Górecki On 28/05/2025 1:16 am, Stefano Stabellini wrote: > On Mon, 26 May 2025, Andrew Cooper wrote: >> On 26/05/2025 6:22 pm, Anthony PERARD wrote: >>> On Mon, May 26, 2025 at 05:45:29PM +0100, Andrew Cooper wrote: >>>> On 20/05/2025 9:52 pm, Andrew Cooper wrote: >>>>> diff --git a/automation/scripts/run-tools-tests b/automation/scripts/run-tools-tests >>>>> index 770e97c3e943..8d7aa8fa5140 100755 >>>>> --- a/automation/scripts/run-tools-tests >>>>> +++ b/automation/scripts/run-tools-tests >>>>> @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > "$xml_out" >>>>> printf '<testsuites name="tools.tests">\n' >> "$xml_out" >>>>> printf ' <testsuite name="tools.tests">\n' >> "$xml_out" >>>>> failed= >>>>> -for dir in "$1"/*; do >>>>> - [ -d "$dir" ] || continue >>>>> - echo "Running test in $dir" >>>>> - printf ' <testcase name="%s">\n' "$dir" >> "$xml_out" >>>>> - ret= >>>>> - for f in "$dir"/*; do >>>>> - [ -f "$f" ] || continue >>>>> - [ -x "$f" ] || continue >>>>> - "$f" 2>&1 | tee /tmp/out >>>>> - ret=$? >>>>> - if [ "$ret" -ne 0 ]; then >>>>> - echo "FAILED: $ret" >>>>> - failed+=" $dir" >>>>> - printf ' <failure type="failure" message="binary %s exited with code %d">\n' "$f" "$ret" >> "$xml_out" >>>>> - # TODO: could use xml escaping... but current tests seems to >>>>> - # produce sane output >>>>> - cat /tmp/out >> "$xml_out" >>>>> - printf ' </failure>\n' >> "$xml_out" >>>>> - else >>>>> - echo "PASSED" >>>>> - fi >>>>> - done >>>>> - if [ -z "$ret" ]; then >>>>> - printf ' <skipped type="skipped" message="no executable test found in %s"/>\n' "$dir" >> "$xml_out" >>>>> +for f in "$1"/*; do >>>>> + if [ -x "$f" ]; then >>>>> + echo "SKIP: $f not executable" >>>>> + continue >>>> This should be ! -x >>>> >>>> I had that hunk in the wrong patch when posting this series. >>> With that fixed: >>> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Thanks. > > >> Thanks. >> >>> But I think there's an issue with the script... >>> >>>>> + "$f" 2>&1 | tee /tmp/out >>>>> + ret=$? >>>>> + if [ "$ret" -ne 0 ]; then >>> Is this checking the correct exit value? It seems that without `set -o >>> pipefail`, we only have the exit value of `tee` which should never fail. >>> But I think we should grab the value of ${PIPESTATUS[0]} to actually >>> read the exit value of $f. >> Hmm yes, I think this needs adjusting. >> >> It turns out there are multiple problems with junit, including the fact >> that putting failures in here doesn't cause the outer job to fail. >> >> The internet suggest having a 'script: grep "<failure" junit.xml' step >> to work around this. >> >> I think that wants to be a separate series. The question is whether to >> do this series first or second. I expect I'm going to need to backport >> all of this work to eventually get XTF back onto the older trees. > > I'll leave the choice to you In which case I'll get this committed now, because it's one useful step forward and reduces my queue a bit. ~Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] CI: Improvements to *-tools-test-* jobs 2025-05-20 20:52 [PATCH 0/3] CI: Improvements to *-tools-test-* jobs Andrew Cooper ` (2 preceding siblings ...) 2025-05-20 20:52 ` [PATCH 3/3] CI: Drop custom handling of tools/tests Andrew Cooper @ 2025-05-21 0:10 ` dmkhn 3 siblings, 0 replies; 14+ messages in thread From: dmkhn @ 2025-05-21 0:10 UTC (permalink / raw) To: Andrew Cooper Cc: Xen-devel, Anthony PERARD, Stefano Stabellini, Marek Marczykowski-Górecki On Tue, May 20, 2025 at 09:52:36PM +0100, Andrew Cooper wrote: > Rearrange tools/tests to be more ameanable to running in CI, and drop the > special casing holding it together. > > > > Andrew Cooper (3): > tools/tests: Drop depriv-fd-checker > tools/tests: Install tests into $(LIBEXEC)/tests > CI: Drop custom handling of tools/tests The code changes look good to me Reviewed-by: Denis Mukhin <dmukhin@ford.com> for the series. > > .gitignore | 1 - > automation/scripts/build | 1 - > automation/scripts/qubes-x86-64.sh | 7 +- > automation/scripts/run-tools-tests | 43 ++- > tools/tests/Makefile | 1 - > tools/tests/cpu-policy/Makefile | 6 +- > tools/tests/depriv/Makefile | 52 --- > tools/tests/depriv/depriv-fd-checker.c | 436 ------------------------- > tools/tests/paging-mempool/Makefile | 6 +- > tools/tests/rangeset/Makefile | 6 +- > tools/tests/resource/Makefile | 6 +- > tools/tests/tsx/Makefile | 6 +- > tools/tests/xenstore/Makefile | 6 +- > 13 files changed, 40 insertions(+), 537 deletions(-) > delete mode 100644 tools/tests/depriv/Makefile > delete mode 100644 tools/tests/depriv/depriv-fd-checker.c > > -- > 2.39.5 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-05-28 0:18 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-20 20:52 [PATCH 0/3] CI: Improvements to *-tools-test-* jobs Andrew Cooper 2025-05-20 20:52 ` [PATCH 1/3] tools/tests: Drop depriv-fd-checker Andrew Cooper 2025-05-26 16:42 ` Anthony PERARD 2025-05-28 0:07 ` Stefano Stabellini 2025-05-20 20:52 ` [PATCH 2/3] tools/tests: Install tests into $(LIBEXEC)/tests Andrew Cooper 2025-05-26 16:56 ` Anthony PERARD 2025-05-28 0:07 ` Stefano Stabellini 2025-05-20 20:52 ` [PATCH 3/3] CI: Drop custom handling of tools/tests Andrew Cooper 2025-05-26 16:45 ` Andrew Cooper 2025-05-26 17:22 ` Anthony PERARD 2025-05-26 17:31 ` Andrew Cooper 2025-05-28 0:16 ` Stefano Stabellini 2025-05-28 0:18 ` Andrew Cooper 2025-05-21 0:10 ` [PATCH 0/3] CI: Improvements to *-tools-test-* jobs dmkhn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.