fix: Use process context manager when running in CLI

[toby-coleman]

Summary

Closes #124, ensuring that the destroy() method gets called when running a process via the CLI.

Changes

• Use context manager for running Process in CLI.

[gemini-code-assist]

Hello @toby-coleman, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses issue #124 by ensuring that the destroy() method of a Process is always called when running a process via the CLI. This is achieved by using a context manager for the Process lifecycle. The changes involve modifying the _run_process function in plugboard/cli/process/__init__.py to use async with process: and updating the unit tests in tests/unit/test_cli.py to assert that __aenter__ and __aexit__ are called.

Highlights

• Context Manager: The Process is now used as an asynchronous context manager to ensure proper initialization and destruction.
• CLI Execution: The CLI execution of a process now guarantees that the destroy() method is called.
• Unit Tests: Unit tests have been updated to verify that the context manager's __aenter__ and __aexit__ methods are called.

Changelog

• plugboard/cli/process/init.py
  • Modified _run_process to use async with process: to ensure destroy() is always called.
  • Removed explicit calls to process.init().
• tests/unit/test_cli.py
  • Updated test_cli_process_run to assert that mock_process.__aenter__ and mock_process.__aexit__ are called.
  • Removed assertion for mock_process.init.assert_called_once().

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A context's embrace,
Ensuring cleanup's grace,
No leaks shall remain.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses issue #124 by ensuring the destroy() method is called when running a process via the CLI using a context manager. The changes look good overall, and the addition of the context manager seems like a good way to ensure proper resource cleanup.

Summary of Findings

• Testing __aenter__ and __aexit__: The tests now assert that __aenter__ and __aexit__ are called, which is good. However, it might be beneficial to add a test case where an exception is raised within the async with block to ensure that __aexit__ is still called.

Merge Readiness

The pull request appears to be in good shape. The use of a context manager is a good approach to ensure that the destroy() method is called. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging. I would recommend adding a test case to verify that __aexit__ is called even when an exception is raised within the async with block.

[Copilot]

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

tests/unit/test_cli.py:29

• [nitpick] Consider asserting that aexit is called with (None, None, None) to ensure the context manager cleans up resources as expected.

mock_process.__aexit__.assert_called_once()

plugboard/cli/process/init.py:42

• [nitpick] Consider adding tests for exception handling within the process context to verify that aexit properly cleans up resources when errors occur.

async with process:

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!