diff --git a/agent-notes/improve-log-attachments.md b/agent-notes/improve-log-attachments.md new file mode 100644 index 0000000..9acc852 --- /dev/null +++ b/agent-notes/improve-log-attachments.md @@ -0,0 +1,19 @@ +# Goal + +Attach the previous session's error log to issue reports when it exists. + +# State + +Collect the existing current error log and its .old sibling through the existing log_paths list. Omit missing files, directories and an unavailable error_log_path. Preserve dialog cancellation, metadata, send scheduling and reporting text. Independently based on integrated master; integration PR #1 has now been merged by other work, so its existing composition CI is on master. No open PR is this branch's base. Rebased after #2-#6 merged: #5 removed `import os` from interface.py, so the file check uses pathlib like #5's code. #6's success test expected a nonexistent log to be attached; it now supplies real errors.log/errors.log.old files, and a second test covers missing logs. + +# Decisions and why + +app_framework renames errors.log to errors.log.old on the next startup. A report submitted after restarting otherwise omits the preceding crash evidence. issue_reporter already accepts multiple paths; no transport or storage changes are needed. The missing-path behavior also works with app_framework's separate failed-logging PR. + +# Surprises + +The report-feedback worktree has separate changes in progress. This branch does not modify it or include its reporting-wording change. + +# Next step + +Read this PR's existing composition CI. File presence checks do not guarantee a file remains available until the sender opens it. No production submission, merge or release. diff --git a/app_elements/interface.py b/app_elements/interface.py index cfba433..a3dcd7d 100644 --- a/app_elements/interface.py +++ b/app_elements/interface.py @@ -68,7 +68,11 @@ def report_issue(*args, **kwargs): report = dlg.get_report() report.application_name = application.name report.application_version = application.version - report.log_paths.append(application.error_log_path) + error_log_path = getattr(application, "error_log_path", "") + if error_log_path: + for path in (error_log_path, "%s.old" % error_log_path): + if Path(path).is_file(): + report.log_paths.append(path) dlg.destroy() try: yield Task(application.issue_reporter.send_report, report) diff --git a/tests/test_report_feedback.py b/tests/test_report_feedback.py index b2130ce..7319baf 100644 --- a/tests/test_report_feedback.py +++ b/tests/test_report_feedback.py @@ -1,5 +1,7 @@ +import tempfile import types import unittest +from pathlib import Path from unittest.mock import Mock, patch from support import load_component @@ -41,7 +43,19 @@ def test_send_failure_warns_without_a_success_message(self): modules["wx_utils.popups"].message_box.assert_not_called() def test_success_keeps_report_metadata_and_confirmation(self): - with load_component("app_elements.interface", **self.application_values()) as (component, app, modules): + with tempfile.TemporaryDirectory() as directory: + error_log = Path(directory) / "errors.log" + error_log.touch() + Path("%s.old" % error_log).touch() + values = self.application_values() + values["error_log_path"] = str(error_log) + self.check_success(values, [str(error_log), "%s.old" % error_log]) + + def test_missing_logs_are_not_attached(self): + self.check_success(self.application_values(), []) + + def check_success(self, values, log_paths): + with load_component("app_elements.interface", **values) as (component, app, modules): dialog, report = self.dialog(modules) with patch.dict("sys.modules", modules): operation = component.report_issue() @@ -50,7 +64,7 @@ def test_success_keeps_report_metadata_and_confirmation(self): operation.send(None) self.assertEqual(report.application_name, app.name) self.assertEqual(report.application_version, app.version) - self.assertEqual(report.log_paths, [app.error_log_path]) + self.assertEqual(report.log_paths, log_paths) dialog.destroy.assert_called_once() modules["wx_utils.popups"].message_box.assert_called_once() modules["wx_utils.popups"].warning_message.assert_not_called()