Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: run common hooks in alphabetical order #236

Merged
merged 1 commit into from
Dec 11, 2023
Merged

feat: run common hooks in alphabetical order #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion apport/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@
opt_path = os.path.dirname(opt_path)

# common hooks
for hook in glob.glob(GENERAL_HOOK_DIR + "/*.py"):
for hook in sorted(glob.glob(GENERAL_HOOK_DIR + "/*.py")):

Check warning on line 1162 in apport/report.py

View check run for this annotation

Codecov / codecov/patch

apport/report.py#L1162 

Added line #L1162 was not covered by tests
if _run_hook(self, ui, hook):
return True

Expand Down
14 changes: 14 additions & 0 deletions tests/integration/test_report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,6 +1260,7 @@
"""\
import sys
def add_info(report):
report['Order'] = f'{report.get("Order", "")} foo'
report['Field1'] = 'Field 1'
report['Field2'] = 'Field 2\\nBla'
if 'Spethial' in report:
Expand All @@ -1277,6 +1278,7 @@
textwrap.dedent(
"""\
def add_info(report):
report['Order'] = f'{report.get("Order", "")} foo1'
report['CommonField1'] = 'CommonField 1'
if report['Package'] == 'commonspethial':
raise StopIteration
Expand All @@ -1292,6 +1294,7 @@
textwrap.dedent(
"""\
def add_info(report):
report['Order'] = f'{report.get("Order", "")} foo2'
report['CommonField2'] = 'CommonField 2'
"""
)
Expand All @@ -1305,6 +1308,7 @@
textwrap.dedent(
"""\
def add_info(report, ui):
report['Order'] = f'{report.get("Order", "")} foo3'
report['CommonField3'] = str(ui)
"""
)
Expand All @@ -1320,6 +1324,7 @@
textwrap.dedent(
"""\
def add_info(report):
report['Order'] = f'{report.get("Order", "")} notme'
report['BadField'] = 'XXX'
"""
)
Expand All @@ -1338,6 +1343,7 @@
"CommonField1",
"CommonField2",
"CommonField3",
"Order",
]
),
"report has required fields",
Expand All @@ -1357,6 +1363,7 @@
"CommonField1",
"CommonField2",
"CommonField3",
"Order",
]
),
"report has required fields",
Expand All @@ -1377,6 +1384,7 @@
"CommonField1",
"CommonField2",
"CommonField3",
"Order",
]
),
"report has required fields",
Expand All @@ -1386,6 +1394,7 @@
self.assertEqual(r["CommonField1"], "CommonField 1")
self.assertEqual(r["CommonField2"], "CommonField 2")
self.assertEqual(r["CommonField3"], "NoninteractiveHookUI()")
self.assertEqual(r["Order"].strip(), "foo1 foo2 foo3 foo")

Check warning on line 1397 in tests/integration/test_report.py

View check run for this annotation

Codecov / codecov/patch

tests/integration/test_report.py#L1397 

Added line #L1397 was not covered by tests

r = apport.report.Report()
r["Package"] = "foo 4.5-6"
Expand All @@ -1402,6 +1411,7 @@
"CommonField1",
"CommonField2",
"CommonField3",
"Order",
]
),
"report has required fields",
Expand All @@ -1410,6 +1420,7 @@
self.assertEqual(r["Field2"], "Field 2\nBla")
self.assertEqual(r["CommonField1"], "CommonField 1")
self.assertEqual(r["CommonField2"], "CommonField 2")
self.assertEqual(r["Order"].strip(), "foo1 foo2 foo3 foo")

Check warning on line 1423 in tests/integration/test_report.py

View check run for this annotation

Codecov / codecov/patch

tests/integration/test_report.py#L1423 

Added line #L1423 was not covered by tests

# test hook abort
r["Spethial"] = "1"
Expand All @@ -1428,6 +1439,7 @@
textwrap.dedent(
"""\
def add_info(report, ui):
report['Order'] = f'{report.get("Order", "")} source_foo'
report['Field1'] = 'Field 1'
report['Field2'] = 'Field 2\\nBla'
if report['Package'] == 'spethial':
Expand All @@ -1452,6 +1464,7 @@
"CommonField1",
"CommonField2",
"CommonField3",
"Order",
]
),
"report has required fields",
Expand All @@ -1461,6 +1474,7 @@
self.assertEqual(r["CommonField1"], "CommonField 1")
self.assertEqual(r["CommonField2"], "CommonField 2")
self.assertEqual(r["CommonField3"], "NoninteractiveHookUI()")
self.assertEqual(r["Order"].strip(), "foo1 foo2 foo3 source_foo")

Check warning on line 1477 in tests/integration/test_report.py

View check run for this annotation

Codecov / codecov/patch

tests/integration/test_report.py#L1477 

Added line #L1477 was not covered by tests

# test hook abort
r["Package"] = "spethial"
Expand Down