Add integration-functional workflow.#73
Merged
wpak-ai merged 16 commits intoMay 28, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI workflow and orchestration script, pytest fixtures and fixture files, container exec helpers, an ephemeral GitHub repo manager, a Weblate REST test client, and end-to-end functional tests for QuickBook, BoostComponentService scanning, and Celery task flows. ChangesIntegration functional test infrastructure
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant Runner
participant Script as integration-functional.sh
participant Compose as Docker Compose
participant Weblate as Weblate container
participant GitHubAPI as GitHub API
participant Pytest
GH->>Runner: start job
Runner->>Script: execute script
Script->>Compose: docker compose up (build & start)
Compose->>Weblate: start services
Script->>Weblate: wait-for-health, create admin token
Script->>GitHubAPI: create ephemeral repo, push fixtures, add deploy key
Script->>Pytest: run tests (passes WEBLATE_API_TOKEN, SSH key)
Pytest->>Weblate: REST calls / in-container execs (create project/component, upload, tasks)
Script->>GH: upload logs on failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
tests/integration/lib/weblate_api.py (1)
246-252: 💤 Low valueInconsistent format specifier for
intervalin template string.Line 248 uses
{interval}while line 239 uses{remaining:.1f}. For consistency and to avoid floating-point representation quirks in the generated code, consider using the same format specifier.Proposed fix
- time.sleep({interval}) + time.sleep({interval:.1f})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/lib/weblate_api.py` around lines 246 - 252, The template string uses inconsistent format specifiers: change the `{interval}` placeholder to match the `{remaining:.1f}` style (e.g., `{interval:.1f}`) so the generated sleep/timeout messaging in the loop in weblate_api.py uses a consistent floating-point format; update the f-string around the time.sleep/TimeoutError/print block (the code that references `interval`, `remaining`, and `task_id`) to use `{interval:.1f}`.tests/integration/lib/gh_repo.py (1)
124-143: 💤 Low valueRemove unused
ownervariable and stale comments.The
ownervariable is resolved but never used. The_ = owneron line 143 silences the linter but the call on line 126 is unnecessary overhead. The comments on lines 127 and 142 suggest incomplete branch-handling logic that was never implemented.Proposed cleanup
def push_fixtures(self, fixture_dir: Path, branch: str) -> None: """Upload fixture files under doc/ on the given branch.""" - owner = self.resolve_owner() - # Create branch from default if needed via initial commit on branch files = [ ("doc/quickbook_fixture.qbk", "quickbook_fixture.qbk"), ("doc/asciidoc_fixture.adoc", "asciidoc_fixture.adoc"), ] for dest, src_name in files: src = fixture_dir / src_name if not src.is_file(): raise FileNotFoundError(src) self._put_file( dest, src.read_bytes(), branch, message=f"Add {dest} for integration tests", ) - # Ensure branch exists as default for clones - _ = owner🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/lib/gh_repo.py` around lines 124 - 143, The push_fixtures function calls resolve_owner() and assigns it to owner but never uses it (and later `_ = owner` only silences the linter) and contains stale comments about branch handling; remove the unnecessary resolve_owner() call and the `_ = owner` no-op, and delete or update the stale comments so push_fixtures contains only the actual fixture upload loop that calls _put_file; if branch creation logic is required later, implement it explicitly in push_fixtures (rather than leaving commented/incomplete notes).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/integration-functional.yml:
- Around line 17-18: The GH_TEST_REPO_TOKEN secret is currently set in the
global env, exposing it to every job/step; move the GH_TEST_REPO_TOKEN: ${{
secrets.GH_TEST_REPO_TOKEN }} entry out of the top-level env and add it only
under the specific step named "Run integration functional tests" (i.e., add an
env block with GH_TEST_REPO_TOKEN under that step and remove the global env
token line), ensuring YAML indentation and workflow syntax remain valid so only
that step receives the secret.
In `@scripts/integration-functional.sh`:
- Line 43: The inline command substitution for WEBLATE_SSH_PUBKEY can hide
failures from compose exec; change the single-line export to first capture the
output into a temp variable (e.g., TMP_WEBLATE_SSH_PUBKEY="$(compose exec -T
weblate cat /app/data/ssh/id_rsa.pub)"), check that TMP_WEBLATE_SSH_PUBKEY is
non-empty (or exit with a clear error) and only then export
WEBLATE_SSH_PUBKEY="$TMP_WEBLATE_SSH_PUBKEY"; this ensures failures in the
compose exec (used to obtain the SSH pubkey) are detected rather than silently
masked when tests in tests/integration/conftest.py later fall back to reading
the file.
In `@tests/integration/conftest.py`:
- Around line 60-63: The test_repo session fixture currently calls pytest.skip
when GH_TEST_REPO_TOKEN is empty, which allows CI to silently skip integration
tests; change that to force CI failure by replacing the skip with a pytest.exit
(or pytest.fail) that stops the run with a clear error message. Locate the token
handling in tests/integration/conftest.py (the session fixture named test_repo
and the GH_TEST_REPO_TOKEN lookup), and when token is falsy call pytest.exit
with a descriptive message like "GH_TEST_REPO_TOKEN not set or secret
misconfigured" (or use pytest.fail) so the pipeline fails instead of skipping
tests.
In `@tests/integration/lib/docker_exec.py`:
- Around line 33-38: The subprocess.run call invoking _compose_cmd(..., _PYTHON,
"-c", snippet) lacks a timeout, so tests can hang; add a timeout argument (e.g.,
timeout=120) to that subprocess.run call and wrap the call in a try/except to
catch subprocess.TimeoutExpired (the call site uses subprocess.run and assigns
to result) so you can set a sensible fallback result/returncode and stderr
message or re-raise a clearer error; ensure you reference the same
subprocess.run invocation and preserve capture_output=True, text=True,
check=False while adding timeout handling.
In `@tests/integration/lib/gh_repo.py`:
- Around line 85-100: The docstring for create_repo() says "Create a private
repo" but the request sets "private": False; either make the repo private by
changing the request body key to "private": True in create_repo() or update the
docstring to accurately state "Create a public repo"; update the description
string returned by the function only if you change semantics, and ensure the
change is applied inside the create_repo method (referenced symbols:
create_repo, self.repo_name, self.repo_full_name).
In `@tests/integration/lib/weblate_api.py`:
- Line 18: Update create_project and create_component to accept 201 Created as a
valid success code in addition to 200 (or explicitly handle an “already exists”
condition) by changing the assertions that check the response status for POST
/api/projects/ and POST /api/projects/{project_slug}/components/ in the
functions create_project and create_component; also fix the formatting in
poll_celery_task where time.sleep is called (replace the malformed
time.sleep({interval}) usage with a proper numeric sleep call and, if printing
remaining time, use the deterministic format string like "{remaining:.1f}" when
formatting the message).
In `@tests/integration/test_functional.py`:
- Around line 100-104: The test_download_translated_qbk currently allows a false
positive by asserting JA_TRANSLATION in text or KNOWN_SOURCE_STRING in text;
change the assertion to require the actual translation: call
weblate_api.download_file(project_slug, component_slug, "ja"), decode as before,
and assert JA_TRANSLATION in text (remove the fallback KNOWN_SOURCE_STRING
check) so the test only passes when the persisted Japanese translation is
present.
- Around line 42-45: Replace the fragile class-attribute state passing with a
scoped pytest fixture: create a module- or class-scoped fixture (e.g.,
created_project_component) that creates the project and component and returns a
context dict/object containing project_slug, component_slug, and task_id; update
tests that currently rely on class attributes
(test_create_project_and_component, test_units_extracted and the code that
sets/reads task_id) to accept this fixture as an argument and use the returned
context instead of reading/writing
type(self).project_slug/component_slug/task_id; refactor the task_id sharing
logic (the block that sets/reads task_id) to write into and read from the
fixture result rather than storing on the test class so tests no longer depend
on execution order.
---
Nitpick comments:
In `@tests/integration/lib/gh_repo.py`:
- Around line 124-143: The push_fixtures function calls resolve_owner() and
assigns it to owner but never uses it (and later `_ = owner` only silences the
linter) and contains stale comments about branch handling; remove the
unnecessary resolve_owner() call and the `_ = owner` no-op, and delete or update
the stale comments so push_fixtures contains only the actual fixture upload loop
that calls _put_file; if branch creation logic is required later, implement it
explicitly in push_fixtures (rather than leaving commented/incomplete notes).
In `@tests/integration/lib/weblate_api.py`:
- Around line 246-252: The template string uses inconsistent format specifiers:
change the `{interval}` placeholder to match the `{remaining:.1f}` style (e.g.,
`{interval:.1f}`) so the generated sleep/timeout messaging in the loop in
weblate_api.py uses a consistent floating-point format; update the f-string
around the time.sleep/TimeoutError/print block (the code that references
`interval`, `remaining`, and `task_id`) to use `{interval:.1f}`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 581a5831-fba4-4695-abac-b2557736318c
📒 Files selected for processing (9)
.github/workflows/integration-functional.ymlscripts/integration-functional.shtests/fixtures/asciidoc_fixture.adoctests/integration/conftest.pytests/integration/lib/__init__.pytests/integration/lib/docker_exec.pytests/integration/lib/gh_repo.pytests/integration/lib/weblate_api.pytests/integration/test_functional.py
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/integration/lib/gh_repo.py (1)
88-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve repository visibility mismatch in
create_repo.Line 88 says “Create a private repo,” but Line 95 sends
"private": False. Please align implementation and docstring so behavior is unambiguous.Proposed fix (docstring-alignment option)
- def create_repo(self) -> str: - """Create a private repo; return SSH clone URL.""" + def create_repo(self) -> str: + """Create a temporary repo; return SSH clone URL."""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/lib/gh_repo.py` around lines 88 - 96, The docstring for create_repo says "Create a private repo" but the request body passes "private": False; update one to match the other: either change the docstring in create_repo to "Create a public repo; return SSH clone URL." to reflect "private": False, or change the request body to "private": True so the implementation creates a private repo; ensure the change is made in the create_repo function where self._request("POST", "/user/repos", body={ "name": self.repo_name, "private": ... , "auto_init": True }) is called and adjust any tests that rely on the repo visibility accordingly.tests/integration/lib/docker_exec.py (1)
67-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a timeout path for
docker_exec_read_file.Line 67 calls
subprocess.runwithout a timeout, so a stuck container can hang the integration suite indefinitely. Please mirrordocker_exec_pythonby adding a timeout (and optionally wrapping timeout expiry with a clearer runtime error).Proposed fix
-def docker_exec_read_file(path: str) -> str: +def docker_exec_read_file(path: str, *, timeout: float = 120.0) -> str: """Read a file from the weblate container.""" - result = subprocess.run( - _compose_cmd("exec", "-T", "weblate", "cat", path), - capture_output=True, - text=True, - check=False, - ) + try: + result = subprocess.run( + _compose_cmd("exec", "-T", "weblate", "cat", path), + timeout=timeout, + capture_output=True, + text=True, + check=False, + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError(f"docker exec cat timed out after {timeout}s: {path}") from exc🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/lib/docker_exec.py` around lines 67 - 72, The subprocess.run call in docker_exec_read_file lacks a timeout and can hang; modify the subprocess.run invocation inside docker_exec_read_file to include the same timeout value used by docker_exec_python, and wrap the call in a try/except that catches subprocess.TimeoutExpired and raises a clearer RuntimeError (including the container name, path, and original timeout) so tests fail fast and give useful context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/integration/lib/docker_exec.py`:
- Around line 67-72: The subprocess.run call in docker_exec_read_file lacks a
timeout and can hang; modify the subprocess.run invocation inside
docker_exec_read_file to include the same timeout value used by
docker_exec_python, and wrap the call in a try/except that catches
subprocess.TimeoutExpired and raises a clearer RuntimeError (including the
container name, path, and original timeout) so tests fail fast and give useful
context.
In `@tests/integration/lib/gh_repo.py`:
- Around line 88-96: The docstring for create_repo says "Create a private repo"
but the request body passes "private": False; update one to match the other:
either change the docstring in create_repo to "Create a public repo; return SSH
clone URL." to reflect "private": False, or change the request body to
"private": True so the implementation creates a private repo; ensure the change
is made in the create_repo function where self._request("POST", "/user/repos",
body={ "name": self.repo_name, "private": ... , "auto_init": True }) is called
and adjust any tests that rely on the repo visibility accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02a516d4-e734-47f6-8139-7ef3ba75bd57
📒 Files selected for processing (5)
tests/integration/conftest.pytests/integration/lib/docker_exec.pytests/integration/lib/gh_repo.pytests/integration/lib/weblate_api.pytests/integration/test_functional.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/integration/test_functional.py
- tests/integration/lib/weblate_api.py
- tests/integration/conftest.py
henry0816191
approved these changes
May 28, 2026
wpak-ai
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #59.
Close #60.
Close #65.
Summary
WeblateAPIREST client,EphemeralGitHubRepohelper for ephemeral GitHub repos with deploy keys, and an AsciiDoc fixture file.github/workflows/integration-functional.yml) withGH_TEST_REPO_TOKENsecret for GitHub-dependent testsTest plan
scripts/integration-functional.shpasses locally against a Docker Compose stackSummary by CodeRabbit
Chores
Tests