Skip to content

🧪 QA: Add unit test for log_event function #326

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

Open
daggerstuff wants to merge 9 commits into
base: staging
Choose a base branch
from
Open

🧪 QA: Add unit test for log_event function #326

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eab7b54
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
b106a0b
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
7934571
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
932211a
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
cb5f965
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
2679c8d
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Mar 31, 2026
3990b81
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Apr 1, 2026
7573285
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Apr 1, 2026
0c2810e
🧪 QA: Add unit test for log_event function
google-labs-jules[bot] Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions workflow/test_worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import unittest
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test file is outside the configured pytest testpaths ("tests" and "ai/core/tests"), so it won’t be collected or run in CI. Move it under the tests/ hierarchy or update pytest testpaths to include workflow/ so the coverage actually runs.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At workflow/test_worker.py, line 1:

<comment>This test file is outside the configured pytest testpaths ("tests" and "ai/core/tests"), so it won’t be collected or run in CI. Move it under the tests/ hierarchy or update pytest testpaths to include workflow/ so the coverage actually runs.</comment>

<file context>
@@ -0,0 +1,29 @@
+import unittest
+import os
+import tempfile
</file context>
Fix with Cubic

import os
import tempfile
import json
import worker
Copy link
Copy Markdown

@charliecreates charliecreates bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workflow/test_worker.py importing worker directly is fragile: depending on how tests are executed (repo root vs workflow/ as CWD), this can import the wrong module or fail. It’s safer to import via the package/module path used by the application (e.g., from workflow import worker or a relative import).

Suggestion

Prefer an absolute/relative package import so the test runs reliably under CI runners invoked from the repo root.

from workflow import worker
# or, if `workflow` is a package:
from . import worker

Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.


class TestWorker(unittest.TestCase):
def setUp(self):
self.fd, self.temp_path = tempfile.mkstemp()
self.original_log_path = worker.LOG_PATH
worker.LOG_PATH = self.temp_path

Comment on lines +9 to +12
Copy link
Copy Markdown

@charliecreates charliecreates bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tempfile.mkstemp() returns an open file descriptor. Keeping it open while worker.log_event() opens/writes the same path can fail on Windows due to file locking/sharing semantics. Close the FD immediately after creating the path (or use NamedTemporaryFile(delete=False) and close it).

Suggestion

Close the mkstemp() descriptor right away (and set fd=None so tearDown doesn’t double-close), or switch to NamedTemporaryFile.

def setUp(self):
    self.fd, self.temp_path = tempfile.mkstemp()
    os.close(self.fd)
    self.fd = None
    self.original_log_path = worker.LOG_PATH
    worker.LOG_PATH = self.temp_path

def tearDown(self):
    worker.LOG_PATH = self.original_log_path
    if self.fd is not None:
        os.close(self.fd)
    os.remove(self.temp_path)

Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.

def tearDown(self):
worker.LOG_PATH = self.original_log_path
os.close(self.fd)
os.remove(self.temp_path)

def test_log_event(self):
worker.log_event("evt_123", "test message")
with open(self.temp_path, "r") as f:
lines = f.readlines()
self.assertEqual(len(lines), 1)
data = json.loads(lines[0])
self.assertEqual(data["event_id"], "evt_123")
self.assertEqual(data["message"], "test message")
self.assertIn("timestamp", data)

if __name__ == "__main__":
unittest.main()
Loading