elastic / elastic/ai-github-actions

[autonomy-atomicity] Shared GraphQL test stub couples two review-thread scripts

Open
#1,758 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
11
Forks
16
Avg merge
22h 9m
Merged PRs (30d)
31

Description

## Autonomy / Atomicity Findings

### 1. Shared test stub forces unrelated workflow changes through one file
**Category:** Over-broad test
**File(s):**
- `tests/test_review_thread_pagination.py`
- `claude-workflows/pr-review/scripts/pr-existing-comments.sh`
- `claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh`

**Problem:**
`tests/test_review_thread_pagination.py` validates two different scripts from different workflow areas in one test module, using a single shared `gh` stub generator (`_write_gh_stub`). That stub contains query-shape branching used by both tests (`tests/test_review_thread_pagination.py:13-147`, especially `:53-55` and `:115-139`).

At the same time, the two scripts request different GraphQL shapes:
- `pr-existing-comments.sh` requests `originalCommit` (`claude-workflows/pr-review/scripts/pr-existing-comments.sh:93-94` and `:134-135`)
- `gh-get-review-threads.sh` does not (`claude-workflows/mention-in-pr/scripts/gh-get-review-threads.sh:50-56` and `:87-91`)

Because both tests depend on one branching stub, a legitimate query/field change for one script often requires touching this same shared fixture and can break the other test path. In parallel development, this creates avoidable merge conflicts and cross-feature red builds in a file unrelated to the second change.

**Suggested fix:**
Split this into per-script fixtures/tests:
1. Keep one test module focused on `pr-existing-comments.sh` with a stub fixture that explicitly includes commit metadata.
2. Keep another module focused on `gh-get-review-threads.sh` with only its required query fields.
3. Share only minimal generic helpers (e.g., `parse_fields`) while separating script-specific response payloads.

## Suggested Actions
- [ ] Split `tests/test_review_thread_pagination.py` into script-specific test modules and fixtures.
- [ ] Remove query-shape branching from a single shared stub so one script’s GraphQL changes do not require edits to the other script’s test path.
- [ ] Keep pagination assertions isolated per script to reduce cross-feature failures during concurrent development.

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Autonomy Atomicity Analyzer](https://github.com/elastic/ai-github-actions/actions/runs/30930210940)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Contributor guide

Open the contributing guide

Research direction

Start with tests/test_review_thread_pagination.py, especially _write_gh_stub and the pagination tests, then compare the GraphQL requests in pr-existing-comments.sh and gh-get-review-threads.sh. Run the existing review-thread pagination tests before separating them into script-specific modules and fixtures. Done means each script has isolated query-shape data and pagination assertions, with only minimal generic helpers shared.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, python, shell
Domain
testing, tooling
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.