Maja's GHA for running tests in CI

[TomasTomecek]

Cheery-picked from #133 so we can get it merged ideally today

[gemini-code-assist]

Code Review

This pull request introduces two GitHub Actions workflows for CI: one for running tests in a container and another for pre-commit checks. The workflows are a great addition for automating quality checks. My review includes a few suggestions to improve their stability and performance:

• In check-in-container.yml, I've noted a potential issue with a missing Makefile.tests file and suggested adding container image caching to speed up builds.
• In pre-commit.yml, I recommend using a stable Python version instead of a pre-release one and adding caching for both pip packages and pre-commit environments to make the job run faster.
  Overall, these are good changes that will improve the development workflow.

[gemini-code-assist]

This step executes make check-in-container. Based on the main Makefile, this target delegates to Makefile.tests ($(MAKE) -f Makefile.tests check-in-container). However, the file Makefile.tests is not included in this pull request or the provided repository files. If this file is missing from the main branch, this workflow step will fail. Please ensure Makefile.tests exists in the repository.

[gemini-code-assist]

This step builds the container image from scratch on every workflow run, which can be inefficient and slow down the CI process. To improve performance, consider implementing caching for the container image layers. A common approach is to use a container registry (like GitHub Container Registry) to store cached layers and use podman build --cache-from and --cache-to options. Alternatively, you could use actions/cache to cache Podman's local storage, although this can be more complex to set up correctly.

[gemini-code-assist]

For better stability and performance, I recommend two changes here:

1. Switch from the pre-release Python 3.13 to a stable version like 3.12.
2. Enable pip caching using the cache input of actions/setup-python. This will speed up dependency installation on subsequent runs.

      uses: actions/setup-python@v4
      with:
        python-version: '3.12'
        cache: 'pip'

[gemini-code-assist]

To further improve performance, I recommend caching the pre-commit environments. This prevents pre-commit from re-creating the hook environments on every run. You can add a caching step before installing pre-commit. The cache key uses hashFiles('.pre-commit-config.yaml'), which assumes the file exists at the root of your repository.

    - name: Cache pre-commit environments
      uses: actions/cache@v4
      with:
        path: ~/.cache/pre-commit
        key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }}
        restore-keys: |
          pre-commit-${{ runner.os }}-

    - name: Install pre-commit
      run: |
        python -m pip install --upgrade pip
        pip install pre-commit

[lbarcziova]

I installed pre-commit-ci, so shouldn't be needed anymore

[lbarcziova]

reverted, see Slack 😅 but will try to come up with changes to the config to let some stuff like patches without changes which would be needed nevertheless, and then reintroduce it

[TomasTomecek]

thanks, makes sense to me!

[TomasTomecek]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces changes to enable running tests in CI. It adds redis as a test dependency, configures git user identity in test fixtures to prevent failures in CI environments, and includes numerous minor whitespace and formatting fixes across various files. The changes are generally good and improve the test setup. I've found one instance of redundant code in the tests that could be removed to improve maintainability.

[gemini-code-assist]

The git_repo fixture, which is used by this test, already configures the git user name and email for the test repository. This block of code is therefore redundant and can be removed.

[nforro]

I think I asked the same question on the original review and Maja answered that it doesn't work without this, which just doesn't make sense to me. I don't see anything that would change the git configuration between git_repo() run and this point.

[TomasTomecek]

she's right, the tests didn't pass for me in the beeai-tests container:

Initialized empty Git repository in /tmp/pytest-of-root/pytest-0/test_git_log_search_tool_found3/repo/.git/
----------------------------------------------------------------------------------------------- Captured stderr setup -----------------------------------------------------------------------------------------------
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint:
hint:   git config --global init.defaultBranch <name>
hint:
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint:
hint:   git branch -m <name>
hint:
hint: Disable this message with "git config set advice.defaultBranchName false"
Author identity unknown

*** Please tell me who you are.

Run

  git config --global user.email "you@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'root@7946a74eac7f.(none)')
============================================================================================== short test summary info ==============================================================================================
ERROR tests/unit/test_tools.py::test_git_patch_creation_tool_success - subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'Initial commit\n\nCVE-2025-12345']' returned non-zero exit status 128.
ERROR tests/unit/test_tools.py::test_git_log_search_tool_found[CVE-2025-12345--Found 1 matching commit(s) for 'CVE-2025-12345'] - subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'Initial commit\n\n
CVE-2025-12345']' returned non-zero exit status 128.
ERROR tests/unit/test_tools.py::test_git_log_search_tool_found[CVE-2025-12346--No matches found for 'CVE-2025-12346'] - subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'Initial commit\n\nCVE-2025-1
2345']' returned non-zero exit status 128.
ERROR tests/unit/test_tools.py::test_git_log_search_tool_found[-RHEL-123456-Found 1 matching commit(s) for 'RHEL-123456'] - subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'Initial commit\n\nCVE-20
25-12345']' returned non-zero exit status 128.
ERROR tests/unit/test_tools.py::test_git_log_search_tool_found[-RHEL-123457-No matches found for 'RHEL-123457'] - subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'Initial commit\n\nCVE-2025-12345']
' returned non-zero exit status 128.

[nforro]

Then I must be missing something 🙂

[nforro]

Please no whitespace changes in patch files.

[nforro]

I think I asked the same question on the original review and Maja answered that it doesn't work without this, which just doesn't make sense to me. I don't see anything that would change the git configuration between git_repo() run and this point.

[TomasTomecek]

@nforro I rebased and dropped 2 commits: the git-config one and the one with whitespace; let's see

[TomasTomecek]

I don't think it's worth it to reuse the shared part of the two GHA jobs. Composite action sounds like an overkill

[TomasTomecek]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces changes to support running tests in a CI environment. It updates the test container to include git configuration and adds redis as a dependency. The Makefiles are refactored to provide more granular test execution targets for 'agents' and 'mcp_server', both locally and within the container. My review focuses on ensuring the test environment is correctly configured and that the commands are suitable for a CI context. I've found a critical issue with the test container setup that will prevent it from building, and a medium-severity issue regarding container run commands that should be adjusted for CI.

[gemini-code-assist]

These git config commands will fail because the git command-line tool is not installed in the container. The dnf install command on lines 4-14 is missing the git-core package. This will cause the container build to fail. Please add git-core to the list of packages to be installed.

[TomasTomecek]

you are partially right but one of the packages brings it as a dep

[gemini-code-assist]

The -it flags for $(CONTAINER_ENGINE) run are intended for interactive sessions that require a TTY. In a non-interactive CI environment, these flags are unnecessary and can cause issues or warnings. It's recommended to remove them for CI jobs.

	$(CONTAINER_ENGINE) run --rm -v $(CURDIR):/src:z --env TEST_TARGET $(TEST_IMAGE) make -f Makefile.tests check-agents
check-mcp-server-in-container:
	$(CONTAINER_ENGINE) run --rm -v $(CURDIR):/src:z --env TEST_TARGET $(TEST_IMAGE) make -f Makefile.tests check-mcp-server

[TomasTomecek]

nah, we get colors with these flags

[nforro]

I don't think it's worth it to reuse the shared part of the two GHA jobs. Composite action sounds like an overkill

But wouldn't a matrix be enough? Something similar to https://github.com/packit/rpm-shim/blob/main/.github/workflows/tests.yml.

[TomasTomecek]

how come I haven't found that sooner @_@