Add Allure test reporting for tracking test progress#443
Add Allure test reporting for tracking test progress#443vtz wants to merge 2 commits intojumpstarter-dev:mainfrom
Conversation
❌ Deploy Preview for jumpstarter-docs failed. Why did it fail? →
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 27 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughIntegrates Allure test reporting: adds allure-pytest dev dependency, Makefile targets to run/generate Allure results, updates CI to run tests with Allure and upload per-matrix results, adds pytest hooks to classify tests and write environment metadata, and decorates selected tests with Allure metadata. Changes
Sequence Diagram(s)mermaid 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 |
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
a470cae to
5ad1c2b
Compare
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