Add Allure test reporting for tracking test progress#443
Open
vtz wants to merge 3 commits intojumpstarter-dev:mainfrom
Open
Add Allure test reporting for tracking test progress#443vtz wants to merge 3 commits intojumpstarter-dev:mainfrom
vtz wants to merge 3 commits intojumpstarter-dev:mainfrom
Conversation
❌ Deploy Preview for jumpstarter-docs failed. Why did it fail? →
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Allure test reporting: CI uploads per-matrix Allure results and consolidates them into a generated Allure report; Makefile and pyproject updated to run tests with allure-pytest; pytest hooks attach suite metadata and write environment properties; selected tests receive Allure annotations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor "GH Actions\nMatrix Runners"
participant "Test Runner\n(make test-allure)"
participant "GHA Artifact Store"
participant "Allure Job\n(consolidation)"
participant "Allure CLI\n(allure-commandline)"
actor "Allure Report\n(uploaded artifact)"
"GH Actions\nMatrix Runners"->>Test Runner\n(make test-allure): run pytest with allure-pytest
Test Runner\n(make test-allure)->>GHA Artifact Store: upload `python/allure-results/` per matrix
"GH Actions\nMatrix Runners"->>Allure Job\n(consolidation): trigger job after tests
Allure Job\n(consolidation)->>GHA Artifact Store: download all `allure-results-*`
Allure Job\n(consolidation)->>Allure CLI\n(allure-commandline): run `allure generate ... --clean` if results present
Allure CLI\n(allure-commandline)->>Allure Job\n(consolidation): produce `allure-report/`
Allure Job\n(consolidation)->>Allure Report\n(uploaded artifact): upload consolidated `allure-report/`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
python/Makefile (1)
127-131: Consider documenting theallureCLI dependency.The
allure-serveandallure-reporttargets require theallureCLI to be installed separately (e.g., vianpm install -g allure-commandlineor system package). This isn't obvious from the Makefile alone.A comment or note in the help text would improve discoverability:
allure-serve: + # Requires: npm install -g allure-commandline allure serve $(ALLURE_RESULTS_DIR)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/Makefile` around lines 127 - 131, Add documentation that the Makefile targets allure-serve and allure-report require the external allure CLI tool; update the Makefile (near the allure-serve/allure-report targets or the help/phony section) to include a short comment noting to install the CLI (e.g., npm install -g allure-commandline or system package) and optionally reference ALLURE_RESULTS_DIR usage so users know the dependency and how to install it before running those targets.python/conftest.py (2)
89-91: Specify encoding when writing the environment file.Using
open()without an explicit encoding relies on the system default, which can vary. This is a minor robustness issue.Proposed fix
- with open(env_file, "w") as f: + with open(env_file, "w", encoding="utf-8") as f: for key, value in props.items(): f.write(f"{key}={value}\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/conftest.py` around lines 89 - 91, The file-write block using with open(env_file, "w") as f: that iterates over props and writes f.write(f"{key}={value}\n") should explicitly specify an encoding to avoid system-default variability; update the open call (where env_file and f are used) to include encoding="utf-8" (or another chosen explicit encoding) so the environment file is written deterministically.
54-58: Path extraction may fail on Windows.Using
"/packages/"as a path delimiter won't match Windows-style paths (e.g.,C:\...\packages\...). Consider usingpathlibfor cross-platform compatibility:Proposed fix
def _extract_package_name(item): - path_str = str(item.path) - if "/packages/" in path_str: - return path_str.split("/packages/")[1].split("/")[0] - return None + parts = item.path.parts + if "packages" in parts: + idx = parts.index("packages") + if idx + 1 < len(parts): + return parts[idx + 1] + return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/conftest.py` around lines 54 - 58, The _extract_package_name function uses a hardcoded "/packages/" string which breaks on Windows; change it to use pathlib: convert item.path to a pathlib.Path (e.g., Path(str(item.path))), inspect its parts to see if "packages" is present, find the index of "packages" in path.parts and return the next part as the package name, otherwise return None; update references to item.path in _extract_package_name to use the Path-based logic so it works cross-platform..github/workflows/python-tests.yaml (1)
109-133: Consider handling empty results gracefully.The
allure-reportjob may fail if no results were uploaded (e.g., all matrix jobs failed before generating Allure output). Theallure generatecommand will error on an empty or missing directory.Suggested improvement
- name: Generate Allure report run: | npm install -g allure-commandline + if [ -d "allure-results" ] && [ "$(ls -A allure-results 2>/dev/null)" ]; then allure generate allure-results -o allure-report --clean + else + echo "No Allure results found, skipping report generation" + exit 0 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/python-tests.yaml around lines 109 - 133, The allure-report job should skip generation/upload when there are no results: after the "Download all Allure results" step (uses: actions/download-artifact@v4) add a check for the existence and non-emptiness of the allure-results directory and only run the "Generate Allure report" (the npm install + allure generate commands) and the subsequent upload step if files are present; otherwise exit gracefully (or create a small placeholder file) so that the allure generate command is never invoked against an empty or missing directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@python/packages/jumpstarter-driver-network/jumpstarter_driver_network/driver_test.py`:
- Line 8: Replace the unconditional "import allure" with a guarded import (try:
import allure except ImportError: allure = None) and update any
usages/decorators in driver_test.py to handle allure being None (either wrap
decorator usage with a helper no-op decorator or use conditional getattr calls,
e.g., use a small helper function no_op_decorator = (lambda *a, **k: (lambda f:
f)) and apply getattr(allure, 'severity', no_op_decorator) or only apply
decorators when allure is not None) so tests run when allure-pytest is not
installed.
In `@python/packages/jumpstarter-testing/jumpstarter_testing/pytest_test.py`:
- Line 1: The file jumpstarter_testing/pytest_test.py unconditionally imports
the allure module which will raise ImportError when allure-pytest isn't
installed; change the top-level import to be conditional (e.g., wrap import
allure in try/except ImportError and set allure = None) and ensure any
module-level use (decorators or calls) is guarded (only apply decorators or call
allure.* when allure is not None), or alternatively use
pytest.importorskip("allure") inside tests to skip Allure-specific behavior when
unavailable.
---
Nitpick comments:
In @.github/workflows/python-tests.yaml:
- Around line 109-133: The allure-report job should skip generation/upload when
there are no results: after the "Download all Allure results" step (uses:
actions/download-artifact@v4) add a check for the existence and non-emptiness of
the allure-results directory and only run the "Generate Allure report" (the npm
install + allure generate commands) and the subsequent upload step if files are
present; otherwise exit gracefully (or create a small placeholder file) so that
the allure generate command is never invoked against an empty or missing
directory.
In `@python/conftest.py`:
- Around line 89-91: The file-write block using with open(env_file, "w") as f:
that iterates over props and writes f.write(f"{key}={value}\n") should
explicitly specify an encoding to avoid system-default variability; update the
open call (where env_file and f are used) to include encoding="utf-8" (or
another chosen explicit encoding) so the environment file is written
deterministically.
- Around line 54-58: The _extract_package_name function uses a hardcoded
"/packages/" string which breaks on Windows; change it to use pathlib: convert
item.path to a pathlib.Path (e.g., Path(str(item.path))), inspect its parts to
see if "packages" is present, find the index of "packages" in path.parts and
return the next part as the package name, otherwise return None; update
references to item.path in _extract_package_name to use the Path-based logic so
it works cross-platform.
In `@python/Makefile`:
- Around line 127-131: Add documentation that the Makefile targets allure-serve
and allure-report require the external allure CLI tool; update the Makefile
(near the allure-serve/allure-report targets or the help/phony section) to
include a short comment noting to install the CLI (e.g., npm install -g
allure-commandline or system package) and optionally reference
ALLURE_RESULTS_DIR usage so users know the dependency and how to install it
before running those targets.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a613c694-e2b9-4971-9e5f-3f08a35dbb13
📒 Files selected for processing (7)
.github/workflows/python-tests.yamlpython/.gitignorepython/Makefilepython/conftest.pypython/packages/jumpstarter-driver-network/jumpstarter_driver_network/driver_test.pypython/packages/jumpstarter-testing/jumpstarter_testing/pytest_test.pypython/pyproject.toml
python/packages/jumpstarter-driver-network/jumpstarter_driver_network/driver_test.py
Outdated
Show resolved
Hide resolved
python/packages/jumpstarter-testing/jumpstarter_testing/pytest_test.py
Outdated
Show resolved
Hide resolved
Integrate allure-pytest to generate rich HTML test reports with automatic test categorization by package and component type (Core, Drivers, CLI, etc.). CI uploads per-matrix results and merges them into a single combined report artifact. Made-with: Cursor
Wrap `import allure` in try/except with a no-op stub so tests run without errors when allure-pytest is not installed (e.g. via `make test` instead of `make test-allure`). Made-with: Cursor
vtz
force-pushed
the
feature/allure-test-reporting
branch
from
a470cae to
5ad1c2b
Compare
- Pin allure-commandline@2.32.0 in CI for reproducible builds - Guard allure-report generation against empty results directory - Clean stale results at the start of test-allure target - Add encoding="utf-8" to environment.properties file write - Document allure CLI requirement in Makefile help and targets
Contributor
Author
|
Addressed review feedback in e0daf5e:
Intentionally skipped: pathlib-based path extraction for Windows compat — CI runs on Linux and macOS only, and the Makefile itself is Unix-only. Adding Windows path handling would be dead code. Note: GitHub Actions workflows are in |
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
1 participant
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.
Summary
allure-pytestinto the test pipeline to generate rich HTML reports with automatic categorization of tests by component (Core, Drivers, CLI, Testing, Kubernetes, Utilities)test-allure,allure-serve,allure-report) that injectallure-pytestviauv run --withwithout changing existing test targetsChanges
python/pyproject.tomlallure-pytest>=2.13.0to workspace dev depspython/conftest.pyenvironment.propertiespython/Makefilepkg-test-allure-%,test-allure,allure-serve,allure-reporttargets.github/workflows/python-tests.yamlmake test-allure, upload results per matrix cell, generate combined reportpython/.gitignoreallure-results/andallure-report/*_test.py(2 files)@allure.feature/@allure.story/@allure.severitydecoratorsDesign decisions
make test/make pkg-test-<pkg>are untouched. Allure is only activated via the newtest-alluretargets.allure-pytestis injected at runtime withuv run --with allure-pytest, so no individual packagepyproject.tomlchanges needed.conftest.pyis guarded bytry/except ImportError, so tests work normally without allure installed.Test plan
cd python && make teststill passes without allure (existing behavior preserved)cd python && make test-alluregenerates results inallure-results/cd python && make allure-serveopens the report in a browserallure-results-*artifacts per matrix joballure-reportjob produces a merged HTML report artifactMade with Cursor