refactor: isolate commit execution in separate module #266
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @andre15silva, I'm Gemini Code Assist1! 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 focuses on refactoring the collect_repos.py and collect_bugs.py files to isolate commit execution into a separate module. This involves creating a new gitbugactions/commit_execution module with submodules for errors, executors, and results. The CommitExecutor class is introduced to handle cloning, checking out commits, applying patches, executing workflows, and cleaning up resources. The changes aim to improve the modularity and maintainability of the codebase, specifically around how tests are executed at specific commits. The collect_repos.py file is modified to use the new CommitExecutor class instead of directly using clone_repo and other functions. The collect_bugs.py file has a minor change to the git clean command.
Highlights
- Commit Execution Isolation: The core change is the isolation of commit execution logic into a new
gitbugactions/commit_executionmodule, enhancing code organization. - CommitExecutor Class: Introduction of the
CommitExecutorclass to encapsulate the process of cloning repositories, checking out commits, applying patches, and executing tests. - collect_repos.py Refactoring: The
collect_repos.pyfile is refactored to utilize theCommitExecutorfor handling repository operations and test executions, replacing direct calls to cloning and cleaning functions. - Error Handling: The new
gitbugactions/commit_execution/errors.pyfile defines custom exception classes for commit execution, patch application, and execution timeouts. - Test Results: The new
gitbugactions/commit_execution/results.pyfile defines data classes for representing test results and commit execution results.
Changelog
Click here to see the changelog
- collect_bugs.py
- Modified the
git cleancommand to use-ffinstead of-ffor more aggressive cleaning.
- Modified the
- collect_repos.py
- Removed direct usage of
clone_repoanddelete_repo_clone. - Introduced
CommitExecutorfor managing repository cloning, checkout, and test execution. - Refactored
handle_repomethod to useCommitExecutorfor executing tests and gathering workflow information. - Added error handling for commit execution and patch application.
- Modified data collection to include information about test execution results, build tools, and workflow status.
- Added logic to get workflow information without executing tests.
- Added a temporary directory to clone the repository.
- Added a method to get the latest commit SHA from the repository.
- Added logic to format test results for backward compatibility.
- Added logic to create actions_run structure for backward compatibility.
- Removed direct usage of
- gitbugactions/commit_execution/init.py
- Created a new module to encapsulate commit execution logic.
- Defined imports for errors, executors, and results submodules.
- Defined all to expose the public API of the module.
- gitbugactions/commit_execution/errors.py
- Defined custom exception classes for commit execution errors, including
CommitExecutionError,ExecutionError,ExecutionTimeoutError, andPatchApplicationError.
- Defined custom exception classes for commit execution errors, including
- gitbugactions/commit_execution/executor.py
- Introduced the
CommitExecutorclass for managing repository operations and test executions. - Implemented methods for cloning repositories, checking out commits, applying patches, and executing workflows.
- Implemented resource cleanup logic.
- Implemented methods for detecting the repository language and getting workflow information.
- Implemented methods for converting ActTestsRun results to a CommitExecutionResult.
- Introduced the
- gitbugactions/commit_execution/results.py
- Defined data classes for representing test results (
TestResult) and commit execution results (CommitExecutionResult).
- Defined data classes for representing test results (
- gitbugactions/test_executor.py
- Modified the
git cleancommand to use-ffinstead of-ffor more aggressive cleaning.
- Modified the
- gitbugactions/utils/actions_utils.py
- Modified the
git cleancommand to use-ffinstead of-ffor more aggressive cleaning.
- Modified the
- test/commit_execution/init.py
- Added a test module for the commit_execution module.
- test/commit_execution/test_commit_executor.py
- Added tests for the CommitExecutor class.
- test/integration_tests/test_collect_repos.py
- Modified assert to check if actions_successful is False instead of None.
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.
Trivia time!
What is the name of the tool used to run GitHub Actions locally?
Click here for the answer
The tool used to run GitHub Actions locally is called Act.
Footnotes
-
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. ↩
There was a problem hiding this comment.
Code Review
The pull request refactors the commit execution process by isolating it into a separate module. This improves modularity and maintainability. The changes include creating a CommitExecutor class to handle cloning, checking out commits, applying patches, and executing tests. The collect_repos.py file is updated to use the new CommitExecutor class. The changes look good overall.
Merge Readiness
The code changes look good and the pull request is ready to be merged. I am unable to directly approve the pull request, and users should have others review and approve this code before merging.
| if not Path(out_path).exists(): | ||
| os.makedirs(out_path, exist_ok=True) | ||
|
|
||
| Act(base_image=base_image) # Initialize Act with base_image |
There was a problem hiding this comment.
Why is this not required anymore?
There was a problem hiding this comment.
Good point. Initializing Act is still required but this should be handled by the commit execution module, specifically the local backend.
The collect_repos script shouldn't deal with such low-level details.
| test_patch: Optional[Union[PatchSet, str]] = None, | ||
| non_code_patch: Optional[Union[PatchSet, str]] = None, |
There was a problem hiding this comment.
Maybe it would be better if this was a List of patches in the execute_at_commit function. This way it would be more flexible and you would not repeat code between functions (if the list is empty, you would not apply any patch).
There was a problem hiding this comment.
Yes, good point. Will make that change.
| Cleans the patch to be used by pygit2. | ||
| This is related to an issue that causes libgit2 to segfault when one of the paths is /dev/null. | ||
| """ | ||
| for file in patch: | ||
| if file.source_file == "/dev/null" and not file.is_added_file: | ||
| file.source_file = file.target_file.replace("b/", "a/", 1) | ||
| elif file.target_file == "/dev/null" and not file.is_removed_file: | ||
| file.target_file = file.source_file.replace("a/", "b/", 1) | ||
| return patch |
There was a problem hiding this comment.
There are new versions of pygit2. Maybe we should check if this is still a problem?
There was a problem hiding this comment.
Yes, we can extract a new issue for later for this
|
@andre15silva how did you prompt Gemini to explain the diff? |
|
It looks like @andre15silva was prompting Gemini to explain the diff by using the |
I guess Gemini already answered ahah. It's this plugin here https://github.com/apps/gemini-code-assist |
|
Thanks @andre15silva and @gemini-code-assist[bot] for the clarification! I appreciate the link to the Gemini Code Assist plugin, I'll check it out. |
No description provided.