test(tools): add unit tests for the four GitLab tools by 0xDevNinja · Pull Request #1003 · Tracer-Cloud/opensre

0xDevNinja · 2026-04-27T12:19:46Z

Fixes #993.

Describe the changes you have made in this PR -

Adds direct unit tests for the four GitLab tools that previously had no coverage under tests/tools/:

tests/tools/test_gitlab_commits_tool.py — list_gitlab_commits
tests/tools/test_gitlab_file_tool.py — get_gitlab_file_contents
tests/tools/test_gitlab_mrs_tool.py — list_gitlab_mrs
tests/tools/test_gitlab_pipelines_tool.py — list_gitlab_pipelines

Each file follows the same shape as tests/tools/test_github_commits_tool.py:

Contract test via BaseToolContract — pulls metadata, schema, and basic is_available / extract_params invariants from tests/tools/conftest.py.
is_available — covers the connection_verified + project_id (and file_path for the file tool) gating, plus the empty-state fallthrough.
extract_params — asserts each field flows from sources["gitlab"] to the tool input dict, including the per-tool defaults (ref_name → main, target_branch → main, ref → main, status → failed).
run happy path — patches _resolve_config and the corresponding app.integrations.gitlab.get_gitlab_* helper to return a fake response, and verifies the tool returns available=True with the expected key (commits, mrs, pipelines, or file).
run error path — patches _resolve_config to return None and verifies the tool returns the documented available=False payload (and an empty-list path for the list tools, mirroring how _request_json coerces non-200 responses to []). For the file tool, also covers the oversize-guard and the binary-file (UnicodeDecodeError) branches.

Each commit is atomic — one file per commit — to keep review focused per tool.

Demo/Screenshot for feature changes and bug fixes -

Local quality gate (clean tree, run from make):

make lint — All checks passed!
make format-check — 908 files already formatted
make typecheck — Success: no issues found in 391 source files
make test-cov — 3363 passed, 2 skipped, 1 xfailed (the four new test files contribute 49 cases)

tests/tools/test_gitlab_commits_tool.py   ............         [12/12]
tests/tools/test_gitlab_file_tool.py      .............        [13/13]
tests/tools/test_gitlab_mrs_tool.py       ............         [12/12]
tests/tools/test_gitlab_pipelines_tool.py ............         [12/12]

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

No, I wrote all the code myself
Yes, I used AI assistance (continue below)

Explain your implementation approach:

I followed the existing GitHub-tool test pattern (tests/tools/test_github_commits_tool.py) so reviewers see one shape repeated four times. The mocking strategy patches the two public surfaces the tool actually depends on — _resolve_config() and the matching get_gitlab_* integration helper — instead of patching httpx directly. That keeps the tests honest to the tool boundary: the tool is responsible for shaping the result dict and gating on a missing config, while the integration layer (already covered separately by app/integrations/gitlab_test.py, tracked by #901) is responsible for HTTP semantics. Where the tool has its own branches that aren't pure pass-through — the file tool's 50_000-byte size guard and the UTF-8 decode guard — I added explicit cases so a future change to those thresholds breaks the test rather than the runtime.

I also kept BaseToolContract as the first thing in each file because it doubles as a "did the @tool decorator wire up correctly" smoke test; if the metadata or is_available signature drifts, the contract checks fail before any tool-specific assertion runs.

Checklist before requesting a review

I have added proper PR title and linked to the issue
I have performed a self-review of my code
I can explain the purpose of every function, class, and logic block I added
I understand why my changes work and have tested them thoroughly
I have considered potential edge cases and how my code handles them
If it is a core feature, I have added thorough tests
My code follows the project's style guidelines and conventions

Covers BaseToolContract metadata, is_available (connection_verified + project_id required), extract_params (full mapping + ref_name default to main), and run() across the three observable paths: unavailable when _resolve_config returns None, happy path returning the integration response under `commits`, and the empty-list graceful path the integration uses for non-200 responses. Refs Tracer-Cloud#993.

Covers BaseToolContract metadata, is_available (project_id and file_path both required), extract_params, and the four run() branches the tool explicitly handles: missing config, oversized files (>50_000 bytes), binary files that cannot be UTF-8 decoded, and the happy path that base64-decodes textual content. Refs Tracer-Cloud#993.

Covers BaseToolContract metadata, is_available, extract_params (full mapping + target_branch default to main), and the three run() paths: unavailable when _resolve_config returns None, happy path returning the integration MR list under `mrs`, and the empty-list graceful path. Refs Tracer-Cloud#993.

Covers BaseToolContract metadata, is_available, extract_params (full mapping + ref default to main + status default to failed), and the three run() paths: unavailable when _resolve_config returns None, happy path returning the integration pipeline list under `pipelines`, and the empty-list graceful path. Refs Tracer-Cloud#993.

greptile-apps · 2026-04-27T12:23:13Z

Greptile Summary

Adds unit tests for all four GitLab tools (list_gitlab_commits, get_gitlab_file_contents, list_gitlab_mrs, list_gitlab_pipelines), following the same shape as the existing GitHub commits tests. Each file covers BaseToolContract, is_available gating, extract_params field mapping with defaults, a config-missing error path, a happy path, and an empty-result path — with GitLabFileTool additionally covering the oversize guard and binary-file UnicodeDecodeError branch. Patch targets correctly address module-level imported names (e.g. app.tools.GitLabFileTool._resolve_config) rather than the original definition site.

Confidence Score: 5/5

Safe to merge — all findings are P2 style suggestions with no correctness impact.

The tests are well-structured, patch targets are correct, and error-message assertions align with the actual implementation strings. The only gap is a missing default-ref test in the file tool, which is a symmetry/style concern and does not affect correctness or runtime behaviour.

tests/tools/test_gitlab_file_tool.py — missing default ref test for completeness

Important Files Changed

Filename	Overview
tests/tools/test_gitlab_commits_tool.py	Adds 6 tests covering contract, is_available gating, extract_params field mapping + default, config-missing error path, happy path, and empty-list path. Patch targets are correct (module-level bindings in GitLabCommitsTool).
tests/tools/test_gitlab_file_tool.py	Adds 7 tests covering contract, is_available, extract_params, config-missing, happy-path base64 decode, oversize guard, binary/UnicodeDecodeError branch, and empty-content path. Patch targets are correct. Missing one default-ref test for symmetry with the other three tools.
tests/tools/test_gitlab_mrs_tool.py	Adds 6 tests covering contract, is_available, extract_params field mapping + default branch, config-missing, happy path, and empty-list path. Patch targets are correct.
tests/tools/test_gitlab_pipelines_tool.py	Adds 6 tests covering contract, is_available, extract_params field mapping (including hardcoded status: failed default) + default ref, config-missing, happy path, and empty-list path. Patch targets are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph "Each GitLab tool test file"
        A["BaseToolContract: metadata + schema + is_available smoke"] --> B["is_available: connection_verified + project_id gating"]
        B --> C["extract_params: field mapping + branch defaults"]
        C --> D["run - config missing: available=False"]
        C --> E["run - happy path: patches _resolve_config + get_gitlab_*: available=True"]
        C --> F["run - empty result: integration returns list: available=True"]
    end
    subgraph "GitLabFileTool extras"
        E --> G["oversized file size > 50000: available=False"]
        E --> H["binary file UnicodeDecodeError: available=False"]
        E --> I["empty content string: available=True"]
    end

_{Reviews (1): Last reviewed commit: "test(tools): add unit tests for GitLabPi..." | Re-trigger Greptile}

greptile-apps · 2026-04-27T12:23:17Z

+            "gitlab": {
+                "connection_verified": True,
+                "project_id": "42",
+                "file_path": "src/main.py",
+                "ref_name": "develop",
+                "gitlab_url": "https://gitlab.example.com",
+                "gitlab_token": "glpat-test",
+            }
+        }
+    )
+    params = rt.extract_params(sources)
+    assert params["project_id"] == "42"
+    assert params["file_path"] == "src/main.py"


Missing default ref test

All three sibling tools include a "defaults X to main" test for their branch parameter (test_extract_params_defaults_ref_name_to_main, test_extract_params_defaults_target_branch_to_main, test_extract_params_defaults_ref_to_main), but the file tool has no equivalent. _get_gitlab_file_extract_params maps gl.get("ref_name", "main") → ref, so when ref_name is absent the default silently stays "main". A test asserting that behaviour would complete the pattern and catch a future threshold change.

def test_extract_params_defaults_ref_to_main() -> None: rt = get_gitlab_file_contents.__opensre_registered_tool__ sources = mock_agent_state( {"gitlab": {"connection_verified": True, "project_id": "42", "file_path": "src/main.py"}} ) params = rt.extract_params(sources) assert params["ref"] == "main"

Good catch — added in 44f6b29. The new test_extract_params_defaults_ref_to_main mirrors the per-tool default-branch tests already present for the other three GitLab tools and locks the ref_name → ref = main default.

Mirrors the per-tool default-branch tests already present for the other three GitLab tools (`test_extract_params_defaults_ref_name_to_main`, `test_extract_params_defaults_target_branch_to_main`, `test_extract_params_defaults_ref_to_main`). Locks `_get_gitlab_file_extract_params` to the documented `ref_name` → `ref` = `main` default so a future threshold change shows up in this file too. Refs Tracer-Cloud#993 (response to Greptile review).

hamzzaaamalik · 2026-04-27T13:32:31Z

Verified locally:

Confirmed all 4 GitLab tools (list_gitlab_commits, get_gitlab_file_contents, list_gitlab_mrs, list_gitlab_pipelines) had no existing tests before this PR
Checked out the branch and ran the new test files: 50/50 pass in 0.36s
Pattern follows the existing BaseToolContract convention used by the GitHub tool tests consistent with the codebase
Pure additive coverage, no production code changes. Looks good to ship pending CI green.

yashksaini-coder

Two issues found — one is a real production bug, the other is a test gap that hides it.

yashksaini-coder · 2026-04-27T16:37:54Z

+                "updated_after": "2026-01-01T00:00:00Z",
+            }
+        }
+    )


This test exposes a production bug in GitLabMRsTool that needs to be fixed first.

In the production code (app/tools/GitLabMRsTool/__init__.py line 18), extract_params does:

"updated_after": gl["updated_after"],

That uses a hard dict key access. If updated_after is missing from sources — which is totally possible since it's not a required field in is_available — this raises a KeyError at runtime and crashes the agent.

The sibling GitLabCommitsTool does it correctly: gl.get("since", ""). Same fix needed here: change to gl.get("updated_after", "") in the production file.

Every test in this file always provides updated_after, so the KeyError is never triggered by the test suite.

Quick fix in GitLabMRsTool/__init__.py:

"updated_after": gl.get("updated_after", ""),

Then add a test calling extract_params without updated_after to verify it defaults to "".

Good catch — fixed in fc55632, default test pinned in 4c69f13. Production code now uses gl.get("updated_after", "") to match the GitLabCommitsTool gl.get("since", "") convention, and the new test_extract_params_defaults_updated_after_to_empty_string exercises the missing-field path. HEAD is 5c545ce.

yashksaini-coder · 2026-04-27T16:37:54Z

+                "project_id": "42",
+                "updated_after": "2026-01-01T00:00:00Z",
+            }
+        }


Same production bug as in GitLabMRsTool — _list_gitlab_pipelines_extract_params in app/tools/GitLabPipelinesTool/__init__.py line 18 also uses gl["updated_after"] (hard key access) instead of gl.get("updated_after", "").

Every test here provides updated_after, so the KeyError is never triggered.

Fix the production code (gl.get("updated_after", "")) and add a test without updated_after in sources to confirm it defaults gracefully.

Same fix applied here — 4dd8f64 defaults updated_after to "" and 5c545ce adds the missing-field test. HEAD is 5c545ce.

Sources may omit updated_after — it is not required by is_available — so gl["updated_after"] raises KeyError at runtime. Match the GitLabCommitsTool convention of gl.get("since", "") and default to an empty string. Refs Tracer-Cloud#1003

Pin the new gl.get("updated_after", "") default so the regression cannot come back silently. Mirrors the pattern used by the target_branch default test. Refs Tracer-Cloud#1003

…ct_params Same hard-key-access bug as GitLabMRsTool: sources may omit updated_after since it is not enforced by is_available, and the dict access raises KeyError at runtime. Default to "" to match the GitLabCommitsTool convention. Refs Tracer-Cloud#1003

…ct_params Pin the new gl.get("updated_after", "") default so the regression cannot come back silently. Mirrors the pattern used by the ref default test. Refs Tracer-Cloud#1003

yashksaini-coder

Both issues are properly addressed — production code fixed and tests added.

Production fixes:

GitLabMRsTool: gl["updated_after"] → gl.get("updated_after", "") ✅
GitLabPipelinesTool: same fix ✅

New tests:

test_extract_params_defaults_updated_after_to_empty_string in both MRs and Pipelines tool tests — directly catches the KeyError regression ✅

Verified locally: 52 tests pass across all 4 tool test files, ruff clean, mypy clean, CI all green. Good to merge.

muddlebee · 2026-04-28T09:07:50Z

LGTM 👍

0xDevNinja added 4 commits April 27, 2026 17:48

greptile-apps Bot reviewed Apr 27, 2026

View reviewed changes

yashksaini-coder requested changes Apr 27, 2026

View reviewed changes

0xDevNinja added 4 commits April 28, 2026 10:33

test(tools): cover missing updated_after in GitLabMRsTool extract_params

4c69f13

Pin the new gl.get("updated_after", "") default so the regression cannot come back silently. Mirrors the pattern used by the target_branch default test. Refs Tracer-Cloud#1003

test(tools): cover missing updated_after in GitLabPipelinesTool extra…

5c545ce

…ct_params Pin the new gl.get("updated_after", "") default so the regression cannot come back silently. Mirrors the pattern used by the ref default test. Refs Tracer-Cloud#1003

0xDevNinja requested a review from yashksaini-coder April 28, 2026 05:06

yashksaini-coder approved these changes Apr 28, 2026

View reviewed changes

yashksaini-coder requested a review from Devesh36 April 28, 2026 07:33

muddlebee merged commit f21bc20 into Tracer-Cloud:main Apr 28, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(tools): add unit tests for the four GitLab tools#1003

test(tools): add unit tests for the four GitLab tools#1003
muddlebee merged 9 commits intoTracer-Cloud:mainfrom
0xDevNinja:issue/993-tests-gitlab-tools

0xDevNinja commented Apr 27, 2026

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Uh oh!

greptile-apps Bot Apr 27, 2026

Uh oh!

0xDevNinja Apr 27, 2026

Uh oh!

hamzzaaamalik commented Apr 27, 2026

Uh oh!

yashksaini-coder left a comment

Uh oh!

yashksaini-coder Apr 27, 2026

Uh oh!

0xDevNinja Apr 28, 2026

Uh oh!

yashksaini-coder Apr 27, 2026

Uh oh!

0xDevNinja Apr 28, 2026

Uh oh!

yashksaini-coder left a comment

Uh oh!

Uh oh!

muddlebee commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

0xDevNinja commented Apr 27, 2026

Describe the changes you have made in this PR -

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Checklist before requesting a review

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

0xDevNinja Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

hamzzaaamalik commented Apr 27, 2026

Uh oh!

yashksaini-coder left a comment

Choose a reason for hiding this comment

Uh oh!

yashksaini-coder Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

0xDevNinja Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

yashksaini-coder Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

0xDevNinja Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

yashksaini-coder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

muddlebee commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants