Merge lp:~gnuoy/charm-helpers/quote-pidof-svc into lp:charm-helpers

Proposed by Liam Young
Status: Merged
Merged at revision: 650
Proposed branch: lp:~gnuoy/charm-helpers/quote-pidof-svc
Merge into: lp:charm-helpers
Diff against target: 118 lines (+21/-16)
2 files modified
charmhelpers/contrib/amulet/utils.py (+1/-1)
tests/contrib/amulet/test_utils.py (+20/-15)
To merge this branch: bzr merge lp:~gnuoy/charm-helpers/quote-pidof-svc
Reviewer Review Type Date Requested Status
David Ames (community) Approve
James Page Approve
Review via email: mp+308381@code.launchpad.net
To post a comment you must log in.
650. By Liam Young

Quote service to examine as it might contain whitespace in systemd land

Revision history for this message
James Page (james-page) :
review: Approve
651. By Liam Young

Update unit tests

Revision history for this message
David Ames (thedac) wrote :

Looks good. Merging.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/amulet/utils.py'
2--- charmhelpers/contrib/amulet/utils.py 2016-07-06 14:41:05 +0000
3+++ charmhelpers/contrib/amulet/utils.py 2016-10-13 13:46:03 +0000
4@@ -546,7 +546,7 @@
5 raise if it is present.
6 :returns: List of process IDs
7 """
8- cmd = 'pidof -x {}'.format(process_name)
9+ cmd = 'pidof -x "{}"'.format(process_name)
10 if not expect_success:
11 cmd += " || exit 0 && exit 1"
12 output, code = sentry_unit.run(cmd)
13
14=== modified file 'tests/contrib/amulet/test_utils.py'
15--- tests/contrib/amulet/test_utils.py 2016-04-05 21:46:12 +0000
16+++ tests/contrib/amulet/test_utils.py 2016-10-13 13:46:03 +0000
17@@ -224,7 +224,7 @@
18 """
19 Normal execution returns a list of pids
20 """
21- self.sentry_unit.commands["pidof -x foo"] = ("123 124 125", 0)
22+ self.sentry_unit.commands['pidof -x "foo"'] = ("123 124 125", 0)
23 result = self.utils.get_process_id_list(self.sentry_unit, "foo")
24 self.assertEqual(["123", "124", "125"], result)
25
26@@ -234,21 +234,21 @@
27 to find a given process results in an amulet.FAIL being
28 raised.
29 """
30- self.sentry_unit.commands["pidof -x foo"] = ("", 1)
31+ self.sentry_unit.commands['pidof -x "foo"'] = ("", 1)
32 with self.assertRaises(SystemExit) as cm, captured_output() as (
33 out, err):
34 self.utils.get_process_id_list(self.sentry_unit, "foo")
35 the_exception = cm.exception
36 self.assertEqual(1, the_exception.code)
37 self.assertEqual(
38- "foo `pidof -x foo` returned 1", out.getvalue().rstrip())
39+ 'foo `pidof -x "foo"` returned 1', out.getvalue().rstrip())
40
41 def test_looks_for_scripts(self):
42 """
43 pidof command uses -x to return a list of pids of scripts
44 """
45 self.sentry_unit.commands["pidof foo"] = ("", 1)
46- self.sentry_unit.commands["pidof -x foo"] = ("123 124 125", 0)
47+ self.sentry_unit.commands['pidof -x "foo"'] = ("123 124 125", 0)
48 result = self.utils.get_process_id_list(self.sentry_unit, "foo")
49 self.assertEqual(["123", "124", "125"], result)
50
51@@ -257,8 +257,10 @@
52 By setting expectation that there are no pids running the logic
53 about when to fail is reversed.
54 """
55- self.sentry_unit.commands["pidof -x foo || exit 0 && exit 1"] = ("", 0)
56- self.sentry_unit.commands["pidof -x bar || exit 0 && exit 1"] = ("", 1)
57+ self.sentry_unit.commands[
58+ 'pidof -x "foo" || exit 0 && exit 1'] = ("", 0)
59+ self.sentry_unit.commands[
60+ 'pidof -x "bar" || exit 0 && exit 1'] = ("", 1)
61 result = self.utils.get_process_id_list(
62 self.sentry_unit, "foo", expect_success=False)
63 self.assertEqual([], result)
64@@ -269,7 +271,7 @@
65 the_exception = cm.exception
66 self.assertEqual(1, the_exception.code)
67 self.assertEqual(
68- "foo `pidof -x bar || exit 0 && exit 1` returned 1",
69+ 'foo `pidof -x "bar" || exit 0 && exit 1` returned 1',
70 out.getvalue().rstrip())
71
72
73@@ -285,8 +287,8 @@
74 PIDs for each unit.
75 """
76 second_sentry = FakeSentry(name="bar")
77- self.sentry_unit.commands["pidof -x foo"] = ("123 124", 0)
78- second_sentry.commands["pidof -x bar"] = ("456 457", 0)
79+ self.sentry_unit.commands['pidof -x "foo"'] = ("123 124", 0)
80+ second_sentry.commands['pidof -x "bar"'] = ("456 457", 0)
81
82 result = self.utils.get_unit_process_ids({
83 self.sentry_unit: ["foo"], second_sentry: ["bar"]})
84@@ -299,8 +301,9 @@
85 Expected failures return empty lists.
86 """
87 second_sentry = FakeSentry(name="bar")
88- self.sentry_unit.commands["pidof -x foo || exit 0 && exit 1"] = ("", 0)
89- second_sentry.commands["pidof -x bar || exit 0 && exit 1"] = ("", 0)
90+ self.sentry_unit.commands[
91+ 'pidof -x "foo" || exit 0 && exit 1'] = ("", 0)
92+ second_sentry.commands['pidof -x "bar" || exit 0 && exit 1'] = ("", 0)
93
94 result = self.utils.get_unit_process_ids(
95 {self.sentry_unit: ["foo"], second_sentry: ["bar"]},
96@@ -320,8 +323,9 @@
97 """
98 We can get the status of a unit.
99 """
100- self.sentry_unit.commands["status-get --format=json --include-data"] = (
101- """{"status": "active", "message": "foo"}""", 0)
102+ self.sentry_unit.commands[
103+ "status-get --format=json --include-data"] = (
104+ """{"status": "active", "message": "foo"}""", 0)
105 self.assertEqual(self.utils.status_get(self.sentry_unit),
106 (u"active", u"foo"))
107
108@@ -330,8 +334,9 @@
109 Older releases of Juju have no status-get command. In those
110 cases we should return the "unknown" status.
111 """
112- self.sentry_unit.commands["status-get --format=json --include-data"] = (
113- "status-get: command not found", 127)
114+ self.sentry_unit.commands[
115+ "status-get --format=json --include-data"] = (
116+ "status-get: command not found", 127)
117 self.assertEqual(self.utils.status_get(self.sentry_unit),
118 (u"unknown", u""))
119

Subscribers

People subscribed via source and target branches