Rework patches for test-path

This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-09-14 10:03:26 +02:00
parent 81cd8d4bcf
commit de06d8e22c
6 changed files with 349 additions and 68 deletions

View File

@ -1,7 +1,7 @@
From a73d30081a13eaeffce87f997726a179ec44d817 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 31 Jul 2020 10:50:37 +0200
Subject: [PATCH 1/2] Revert "test-path: increase timeout"
Subject: [PATCH 1/4] Revert "test-path: increase timeout"
This partially reverts commit 500727c220354b81b68ed6667d9a6f0fafe3ba19.

View File

@ -1,53 +0,0 @@
From a2deeaeaa90d493ef8a2b20656745cd0531a1b30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 31 Jul 2020 10:36:57 +0200
Subject: [PATCH 2/2] test-path: do not fail the test if we fail to start some
service
The test was failing because it couldn't start the service:
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
Failed to connect to system bus: No such file or directory
-.slice: Failed to enable/disable controllers on cgroup /system.slice/kojid.service, ignoring: Permission denied
path-modified.service: Failed to create cgroup /system.slice/kojid.service/path-modified.service: Permission denied
path-modified.service: Failed to attach to cgroup /system.slice/kojid.service/path-modified.service: No such file or directory
path-modified.service: Failed at step CGROUP spawning /bin/true: No such file or directory
path-modified.service: Main process exited, code=exited, status=219/CGROUP
path-modified.service: Failed with result 'exit-code'.
Test timeout when testing path-modified.path
Let's just ignore the failure here. Services can occasionally fail to start,
there's not much we can do in that case.
---
src/test/test-path.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/test/test-path.c b/src/test/test-path.c
index 63b709c8da..6c0db53f10 100644
--- a/src/test/test-path.c
+++ b/src/test/test-path.c
@@ -98,6 +98,14 @@ static void check_states(Manager *m, Path *path, Service *service, PathState pat
service_state_to_string(service->state),
service_result_to_string(service->result));
+ if (service->state == SERVICE_FAILED) {
+ log_warning("Failed to start service %s, ignoring: %s/%s",
+ UNIT(service)->id,
+ service_state_to_string(service->state),
+ service_result_to_string(service->result));
+ break;
+ }
+
if (now(CLOCK_MONOTONIC) >= end) {
log_error("Test timeout when testing %s", UNIT(path)->id);
exit(EXIT_FAILURE);

View File

@ -1,7 +1,7 @@
From 35fbc6b2db3fda9015ecfaa2a60d1a47de35c583 Mon Sep 17 00:00:00 2001
From 4c38dcdc8d8f22dddc521faedad6a4f45fa81d63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Mon, 14 Sep 2020 08:56:28 +0200
Subject: [PATCH] test-path: more debugging information
Subject: [PATCH 2/4] test-path: more debugging information
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
@ -14,7 +14,7 @@ independent parts in log files.
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/src/test/test-path.c b/src/test/test-path.c
index 6c0db53f10..7c32e0948f 100644
index 63b709c8da..84dcf5e37d 100644
--- a/src/test/test-path.c
+++ b/src/test/test-path.c
@@ -1,7 +1,6 @@
@ -25,7 +25,7 @@ index 6c0db53f10..7c32e0948f 100644
#include <sys/stat.h>
#include <sys/types.h>
@@ -78,25 +77,30 @@ static Service *service_for_path(Manager *m, Path *path, const char *service_nam
@@ -78,32 +77,38 @@ static Service *service_for_path(Manager *m, Path *path, const char *service_nam
return SERVICE(service_unit);
}
@ -65,12 +65,6 @@ index 6c0db53f10..7c32e0948f 100644
+ service_state_to_string(service->state),
+ service_result_to_string(service->result));
if (service->state == SERVICE_FAILED) {
log_warning("Failed to start service %s, ignoring: %s/%s",
@@ -106,12 +110,13 @@ static void check_states(Manager *m, Path *path, Service *service, PathState pat
break;
}
- if (now(CLOCK_MONOTONIC) >= end) {
+ if (n >= end) {
log_error("Test timeout when testing %s", UNIT(path)->id);

View File

@ -0,0 +1,245 @@
From 67c6ff720796bc97f262ba93c6ea87da93b04a1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 31 Jul 2020 10:36:57 +0200
Subject: [PATCH 3/4] test-path: do not fail the test if we fail to start some
service
The test was failing because it couldn't start the service:
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
path-modified.path: state = waiting; result = success
path-modified.service: state = failed; result = exit-code
Failed to connect to system bus: No such file or directory
-.slice: Failed to enable/disable controllers on cgroup /system.slice/kojid.service, ignoring: Permission denied
path-modified.service: Failed to create cgroup /system.slice/kojid.service/path-modified.service: Permission denied
path-modified.service: Failed to attach to cgroup /system.slice/kojid.service/path-modified.service: No such file or directory
path-modified.service: Failed at step CGROUP spawning /bin/true: No such file or directory
path-modified.service: Main process exited, code=exited, status=219/CGROUP
path-modified.service: Failed with result 'exit-code'.
Test timeout when testing path-modified.path
In fact any of the services that we try to start may fail, especially
considering that we're doing some rogue cgroup operations. See
https://github.com/systemd/systemd/pull/16603#issuecomment-679133641.
---
src/test/test-path.c | 88 ++++++++++++++++++++++++++++++--------------
1 file changed, 61 insertions(+), 27 deletions(-)
diff --git a/src/test/test-path.c b/src/test/test-path.c
index 84dcf5e37d..d6c37b77e6 100644
--- a/src/test/test-path.c
+++ b/src/test/test-path.c
@@ -77,8 +77,8 @@ static Service *service_for_path(Manager *m, Path *path, const char *service_nam
return SERVICE(service_unit);
}
-static void _check_states(unsigned line,
- Manager *m, Path *path, Service *service, PathState path_state, ServiceState service_state) {
+static int _check_states(unsigned line,
+ Manager *m, Path *path, Service *service, PathState path_state, ServiceState service_state) {
assert_se(m);
assert_se(service);
@@ -102,11 +102,20 @@ static void _check_states(unsigned line,
service_state_to_string(service->state),
service_result_to_string(service->result));
+ if (service->state == SERVICE_FAILED)
+ return log_notice_errno(SYNTHETIC_ERRNO(ECANCELED),
+ "Failed to start service %s, aborting test: %s/%s",
+ UNIT(service)->id,
+ service_state_to_string(service->state),
+ service_result_to_string(service->result));
+
if (n >= end) {
log_error("Test timeout when testing %s", UNIT(path)->id);
exit(EXIT_FAILURE);
}
}
+
+ return 0;
}
#define check_states(...) _check_states(__LINE__, __VA_ARGS__)
@@ -124,18 +133,22 @@ static void test_path_exists(Manager *m) {
service = service_for_path(m, path, NULL);
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(touch(test_path) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
/* Service restarts if file still exists */
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(unit_stop(unit) >= 0);
}
@@ -154,18 +167,22 @@ static void test_path_existsglob(Manager *m) {
service = service_for_path(m, path, NULL);
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(touch(test_path) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
/* Service restarts if file still exists */
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(unit_stop(unit) >= 0);
}
@@ -185,23 +202,28 @@ static void test_path_changed(Manager *m) {
service = service_for_path(m, path, NULL);
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(touch(test_path) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
/* Service does not restart if file still exists */
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
f = fopen(test_path, "w");
assert_se(f);
fclose(f);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
(void) rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL);
assert_se(unit_stop(unit) >= 0);
@@ -222,23 +244,28 @@ static void test_path_modified(Manager *m) {
service = service_for_path(m, path, NULL);
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(touch(test_path) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
/* Service does not restart if file still exists */
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
f = fopen(test_path, "w");
assert_se(f);
fputs("test", f);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
(void) rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL);
assert_se(unit_stop(unit) >= 0);
@@ -258,14 +285,17 @@ static void test_path_unit(Manager *m) {
service = service_for_path(m, path, "path-mycustomunit.service");
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(touch(test_path) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(unit_stop(unit) >= 0);
}
@@ -286,22 +316,26 @@ static void test_path_directorynotempty(Manager *m) {
assert_se(access(test_path, F_OK) < 0);
assert_se(unit_start(unit) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
/* MakeDirectory default to no */
assert_se(access(test_path, F_OK) < 0);
assert_se(mkdir_p(test_path, 0755) >= 0);
assert_se(touch(strjoina(test_path, "test_file")) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
/* Service restarts if directory is still not empty */
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING);
+ if (check_states(m, path, service, PATH_RUNNING, SERVICE_RUNNING) < 0)
+ return;
assert_se(rm_rf(test_path, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
assert_se(unit_stop(UNIT(service)) >= 0);
- check_states(m, path, service, PATH_WAITING, SERVICE_DEAD);
+ if (check_states(m, path, service, PATH_WAITING, SERVICE_DEAD) < 0)
+ return;
assert_se(unit_stop(unit) >= 0);
}

View File

@ -0,0 +1,94 @@
From 1a83d7234e374e991235f4ef21c56998f93cb875 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Mon, 14 Sep 2020 08:58:54 +0200
Subject: [PATCH 4/4] test-path: use Type=exec
In general, Type=exec is superior to Type=simple. Let's not assume that
the service is started before it was really started.
---
test/test-path/path-changed.service | 2 +-
test/test-path/path-directorynotempty.service | 2 +-
test/test-path/path-exists.service | 2 +-
test/test-path/path-existsglob.service | 2 +-
test/test-path/path-makedirectory.service | 2 +-
test/test-path/path-modified.service | 2 +-
test/test-path/path-mycustomunit.service | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/test/test-path/path-changed.service b/test/test-path/path-changed.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-changed.service
+++ b/test/test-path/path-changed.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-directorynotempty.service b/test/test-path/path-directorynotempty.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-directorynotempty.service
+++ b/test/test-path/path-directorynotempty.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-exists.service b/test/test-path/path-exists.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-exists.service
+++ b/test/test-path/path-exists.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-existsglob.service b/test/test-path/path-existsglob.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-existsglob.service
+++ b/test/test-path/path-existsglob.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-makedirectory.service b/test/test-path/path-makedirectory.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-makedirectory.service
+++ b/test/test-path/path-makedirectory.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-modified.service b/test/test-path/path-modified.service
index fb465d76bb..b75552df4f 100644
--- a/test/test-path/path-modified.service
+++ b/test/test-path/path-modified.service
@@ -3,5 +3,5 @@ Description=Service Test for Path units
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true
diff --git a/test/test-path/path-mycustomunit.service b/test/test-path/path-mycustomunit.service
index bcdafe4f30..8fbc40d13f 100644
--- a/test/test-path/path-mycustomunit.service
+++ b/test/test-path/path-mycustomunit.service
@@ -3,5 +3,5 @@ Description=Service Test Path Unit
[Service]
ExecStart=/bin/true
-Type=simple
+Type=exec
RemainAfterExit=true

View File

@ -71,11 +71,12 @@ GIT_DIR=../../src/systemd/.git git diffab -M v233..master@{2017-06-15} -- hwdb/[
Patch0001: use-bfq-scheduler.patch
Patch0002: 0001-Revert-test-path-increase-timeout.patch
Patch0003: 0002-test-path-do-not-fail-the-test-if-we-fail-to-start-s.patch
Patch0004: 0001-test-path-more-debugging-information.patch
Patch0003: 0002-test-path-more-debugging-information.patch
Patch0004: 0003-test-path-do-not-fail-the-test-if-we-fail-to-start-s.patch
Patch0005: 0004-test-path-use-Type-exec.patch
Patch0005: 0001-test-acl-util-output-more-debug-info.patch
Patch0006: 0001-Do-not-assert-in-test_add_acls_for_user.patch
Patch0006: 0001-test-acl-util-output-more-debug-info.patch
Patch0007: 0001-Do-not-assert-in-test_add_acls_for_user.patch
%ifarch %{ix86} x86_64 aarch64
%global have_gnu_efi 1