Add QualityFlow coverage instrumentation (python)#4750
Add QualityFlow coverage instrumentation (python)#4750BishopMaple wants to merge 2 commits intoRedHatQE:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR introduces comprehensive code coverage measurement and reporting infrastructure. It adds a coverage configuration file, configures Docker for coverage instrumentation across all Python processes, implements an HTTP server to expose coverage metrics, and establishes a GitHub Actions workflow to collect and upload coverage data to QualityFlow. ChangesCoverage Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
for more information, see https://pre-commit.ci
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 @.coveragerc:
- Line 1: The PR description is missing required template sections; update the
pull request description to include the full PR template headings exactly:
"##### What this PR does / why we need it:" with a meaningful explanation (not
just "Automated coverage onboarding"), "##### Which issue(s) this PR fixes:"
(can be empty or reference issue keys), "##### Special notes for reviewer:" (can
be empty or include testing/edge cases), and "##### jira-ticket:" (can be empty
or include the ticket), then provide concrete, concise content under each
heading so the PR meets the repository guidelines.
In @.github/workflows/coverage-upload.yml:
- Around line 37-41: The curl invocation in the workflow uses the insecure flag
(-k/--insecure) which disables TLS verification and exposes QUALITYFLOW_API_KEY
and coverage.lcov; remove the -k flag from the curl command in the
coverage-upload.yml step and rely on standard TLS validation (or configure a
trusted CA/certificate bundle via --cacert if necessary) so the POST to the
QualityFlow upload endpoint validates the server certificate while still sending
the X-API-Key and --data-binary `@coverage.lcov` securely.
- Around line 27-33: The "Run tests with coverage" step should be changed so the
pytest failure doesn't prevent generation of coverage.lcov: set
continue-on-error: true on the step named "Run tests with coverage" and split
the step's commands so they run separately (e.g., run pytest under coverage
first, then always run `coverage lcov -o coverage.lcov`) so a partial/empty
coverage.lcov is produced even when tests fail; ensure the step keeps its name
"Run tests with coverage" and then leave the "Upload coverage to QualityFlow"
step as-is (it will now find coverage.lcov) or alternatively add an existence
check before uploading.
In `@coverport/coverage_server.py`:
- Around line 42-43: The log_message method overrides BaseHTTPRequestHandler's
method but uses a parameter name 'format' that shadows the built-in Python
function format. Rename the 'format' parameter to 'fmt' (or another
non-shadowing name) in the method signature along with adding a matching type
annotation, and keep the method body as is to suppress request logs.
- Around line 31-36: Replace the bare "except Exception as e" in the
report-generation try block with explicit exception handlers: catch ImportError
(if coverage import fails), OSError (file I/O problems) and, if coverage was
imported at module level, coverage.CoverageException (report generation errors);
update the response logging and message in each handler to include the specific
exception details and keep the same HTTP 500 response flow (this change touches
the except block that writes to self.wfile and calls
self.send_response/self.send_header/self.end_headers).
- Around line 22-25: The temp file handling leaks and the read uses an unclosed
handle; fix by creating the temp file with tempfile.NamedTemporaryFile(mode="w",
suffix=".lcov", delete=False) but wrap the entire use in a try/finally so the
file is always unlinked, call cov.lcov_report(outfile=f.name) while the temp
exists, exit the NamedTemporaryFile context to close it, then open it with a
with open(f.name) as rf: data = rf.read() to ensure the read handle is closed,
and put os.unlink(f.name) in the finally block to guarantee cleanup if
cov.lcov_report raises; reference tempfile.NamedTemporaryFile, cov.lcov_report,
and the subsequent open/unlink operations when applying the change.
- Around line 13-20: Move the imports out of the request handler: at module top
perform a guarded import for the optional "coverage" package (e.g., try/except
ImportError and assign a module-level name like coverage_module or None) and
import "tempfile" normally at top-level; then update do_GET to use the
module-level names (e.g., check coverage_module and call
coverage_module.Coverage.current()) instead of doing in-function imports. Ensure
references in do_GET use the renamed module variable and handle the case where
coverage is not available.
In `@Dockerfile`:
- Line 72: The ENV line sets PYTHONPATH with a trailing colon which expands to
an empty component and adds the CWD to sys.path; change the ENV assignment for
PYTHONPATH (the ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" line) to a direct
assignment without the expansion so it becomes PYTHONPATH="/app/coverport" to
avoid inserting the current working directory into Python's sys.path.
- Around line 68-73: The COVERAGE_PROCESS_START env var is pointing to
/app/.coveragerc which doesn’t exist; update the Dockerfile to set
COVERAGE_PROCESS_START to the actual copied location using the existing ARG
TEST_DIR (which contains the path where .coveragerc was copied by COPY
--from=builder ${TEST_DIR}/ ${TEST_DIR}/). Find the COVERAGE_PROCESS_START
assignment and change it to reference TEST_DIR (e.g., use ENV
COVERAGE_PROCESS_START="${TEST_DIR}/.coveragerc") so coverage uses the real
.coveragerc at runtime.
- Line 70: The RUN pip install line installs coverage into the system Python
instead of the project's uv virtualenv; change it to use the project's
uv-managed installer (use the same command pattern as the other installs, e.g.,
"uv pip install ...") so coverage is installed into the uv venv used by uv run
pytest, and pin a specific version like coverage==7.6.1 to make the build
reproducible and satisfy Hadolint DL3013; update the RUN invocation that
currently reads "RUN pip install --no-cache-dir coverage>=7.0" accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 470d2636-ba22-4fcf-9fa7-573c95ce0321
📒 Files selected for processing (5)
.coveragerc.github/workflows/coverage-upload.ymlDockerfilecoverport/coverage_server.pycoverport/sitecustomize.py
| @@ -0,0 +1,16 @@ | |||
| [run] | |||
There was a problem hiding this comment.
HIGH: PR description is missing all required template sections.
The description for this automated PR lacks every required section from the PR template:
##### What this PR does / why we need it:— must be present with meaningful content##### Which issue(s) this PR fixes:— must be present (may be empty)##### Special notes for reviewer:— must be present (may be empty)##### jira-ticket:— must be present (may be empty)
Please restore the missing sections. As per coding guidelines, What this PR does / why we need it: must have meaningful content — "Automated coverage onboarding" alone is insufficient.
🤖 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 @.coveragerc at line 1, The PR description is missing required template
sections; update the pull request description to include the full PR template
headings exactly: "##### What this PR does / why we need it:" with a meaningful
explanation (not just "Automated coverage onboarding"), "##### Which issue(s)
this PR fixes:" (can be empty or reference issue keys), "##### Special notes for
reviewer:" (can be empty or include testing/edge cases), and "#####
jira-ticket:" (can be empty or include the ticket), then provide concrete,
concise content under each heading so the PR meets the repository guidelines.
| - name: Run tests with coverage | ||
| run: | | ||
| coverage run -m pytest | ||
| coverage lcov -o coverage.lcov | ||
|
|
||
| - name: Upload coverage to QualityFlow | ||
| if: always() |
There was a problem hiding this comment.
Coverage upload will fail with a cryptic error when tests fail.
The "Run tests with coverage" step (line 29) will exit non-zero if pytest fails, meaning coverage lcov -o coverage.lcov (line 30) never executes and coverage.lcov is never created. The upload step runs unconditionally (if: always()), and curl --data-binary @coverage.lcov`` will error because the file does not exist — obscuring the original test failure.
Guard the upload on the file's existence:
- if: always()
+ if: always() && hashFiles('coverage.lcov') != ''Or add continue-on-error: true to the test step and split the two commands so coverage lcov can still emit a partial report after a partial run.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run tests with coverage | |
| run: | | |
| coverage run -m pytest | |
| coverage lcov -o coverage.lcov | |
| - name: Upload coverage to QualityFlow | |
| if: always() | |
| - name: Run tests with coverage | |
| run: | | |
| coverage run -m pytest | |
| coverage lcov -o coverage.lcov | |
| - name: Upload coverage to QualityFlow | |
| if: always() && hashFiles('coverage.lcov') != '' |
🤖 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 @.github/workflows/coverage-upload.yml around lines 27 - 33, The "Run tests
with coverage" step should be changed so the pytest failure doesn't prevent
generation of coverage.lcov: set continue-on-error: true on the step named "Run
tests with coverage" and split the step's commands so they run separately (e.g.,
run pytest under coverage first, then always run `coverage lcov -o
coverage.lcov`) so a partial/empty coverage.lcov is produced even when tests
fail; ensure the step keeps its name "Run tests with coverage" and then leave
the "Upload coverage to QualityFlow" step as-is (it will now find coverage.lcov)
or alternatively add an existence check before uploading.
| curl -sk -X POST \ | ||
| -H "Content-Type: text/plain" \ | ||
| -H "X-API-Key: $QUALITYFLOW_API_KEY" \ | ||
| --data-binary @coverage.lcov \ | ||
| "https://qualityflow-dashboard-cnv-quality-flow.apps.cnv2.engineering.redhat.com/api/coverage/upload?org=RedHatQE&repo=openshift-virtualization-tests&commit=${{ github.sha }}&branch=${{ github.ref_name }}" |
There was a problem hiding this comment.
CRITICAL: curl -k disables TLS certificate verification while transmitting the API key.
--insecure (-k) on line 37 allows a MITM attacker to intercept the connection, exposing QUALITYFLOW_API_KEY and the coverage payload. Remove it — the server's certificate should be valid.
🔒 Proposed fix
- curl -sk -X POST \
+ curl -s -X POST \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -sk -X POST \ | |
| -H "Content-Type: text/plain" \ | |
| -H "X-API-Key: $QUALITYFLOW_API_KEY" \ | |
| --data-binary @coverage.lcov \ | |
| "https://qualityflow-dashboard-cnv-quality-flow.apps.cnv2.engineering.redhat.com/api/coverage/upload?org=RedHatQE&repo=openshift-virtualization-tests&commit=${{ github.sha }}&branch=${{ github.ref_name }}" | |
| curl -s -X POST \ | |
| -H "Content-Type: text/plain" \ | |
| -H "X-API-Key: $QUALITYFLOW_API_KEY" \ | |
| --data-binary `@coverage.lcov` \ | |
| "https://qualityflow-dashboard-cnv-quality-flow.apps.cnv2.engineering.redhat.com/api/coverage/upload?org=RedHatQE&repo=openshift-virtualization-tests&commit=${{ github.sha }}&branch=${{ github.ref_name }}" |
🤖 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 @.github/workflows/coverage-upload.yml around lines 37 - 41, The curl
invocation in the workflow uses the insecure flag (-k/--insecure) which disables
TLS verification and exposes QUALITYFLOW_API_KEY and coverage.lcov; remove the
-k flag from the curl command in the coverage-upload.yml step and rely on
standard TLS validation (or configure a trusted CA/certificate bundle via
--cacert if necessary) so the POST to the QualityFlow upload endpoint validates
the server certificate while still sending the X-API-Key and --data-binary
`@coverage.lcov` securely.
| def do_GET(self): | ||
| if self.path == "/coverage": | ||
| try: | ||
| import coverage | ||
|
|
||
| cov = coverage.Coverage.current() | ||
| if cov: | ||
| import tempfile |
There was a problem hiding this comment.
HIGH: import coverage and import tempfile inside a request handler violate the "no imports inside functions" guideline.
Both imports belong at the top of the module. Per coding guidelines: "Imports always at the top of the module - do not import inside functions."
import tempfile has no reason to be inside the handler at all. import coverage is an optional dependency — handle it at module level with a guarded import:
♻️ Proposed fix
At the top of the module, replace the bare import with a guarded block:
import http.server
import os
+import tempfile
import threading
+
+try:
+ import coverage as _coverage_module
+ _COVERAGE_AVAILABLE = True
+except ImportError:
+ _COVERAGE_AVAILABLE = FalseThen in do_GET:
def do_GET(self):
if self.path == "/coverage":
try:
- import coverage
-
- cov = coverage.Coverage.current()
+ cov = _coverage_module.Coverage.current() if _COVERAGE_AVAILABLE else None
if cov:
- import tempfile
-
...🤖 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 `@coverport/coverage_server.py` around lines 13 - 20, Move the imports out of
the request handler: at module top perform a guarded import for the optional
"coverage" package (e.g., try/except ImportError and assign a module-level name
like coverage_module or None) and import "tempfile" normally at top-level; then
update do_GET to use the module-level names (e.g., check coverage_module and
call coverage_module.Coverage.current()) instead of doing in-function imports.
Ensure references in do_GET use the renamed module variable and handle the case
where coverage is not available.
| with tempfile.NamedTemporaryFile(mode="w", suffix=".lcov", delete=False) as f: | ||
| cov.lcov_report(outfile=f.name) | ||
| data = open(f.name).read() | ||
| os.unlink(f.name) |
There was a problem hiding this comment.
HIGH: Unclosed file handle on line 24, and temp file leaks if lcov_report raises.
Two issues here:
open(f.name).read()— nowithstatement; the file handle is never explicitly closed, violating: "ALWAYS usewithfor resources — files, connections, locks MUST use context managers."os.unlink(f.name)is inside thewith NamedTemporaryFile(...)block — ifcov.lcov_report(outfile=f.name)raises an exception,os.unlinkis never reached and the temp file is leaked. The outerexcept Exceptioncatches the exception but the file remains on disk.
🐛 Proposed fix
- with tempfile.NamedTemporaryFile(mode="w", suffix=".lcov", delete=False) as f:
- cov.lcov_report(outfile=f.name)
- data = open(f.name).read()
- os.unlink(f.name)
+ with tempfile.NamedTemporaryFile(mode="w", suffix=".lcov", delete=False) as f:
+ tmp_path = f.name
+ try:
+ cov.lcov_report(outfile=tmp_path)
+ with open(tmp_path) as lcov_file:
+ data = lcov_file.read()
+ finally:
+ os.unlink(tmp_path)🧰 Tools
🪛 Ruff (0.15.12)
[warning] 24-24: Use a context manager for opening files
(SIM115)
🤖 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 `@coverport/coverage_server.py` around lines 22 - 25, The temp file handling
leaks and the read uses an unclosed handle; fix by creating the temp file with
tempfile.NamedTemporaryFile(mode="w", suffix=".lcov", delete=False) but wrap the
entire use in a try/finally so the file is always unlinked, call
cov.lcov_report(outfile=f.name) while the temp exists, exit the
NamedTemporaryFile context to close it, then open it with a with open(f.name) as
rf: data = rf.read() to ensure the read handle is closed, and put
os.unlink(f.name) in the finally block to guarantee cleanup if cov.lcov_report
raises; reference tempfile.NamedTemporaryFile, cov.lcov_report, and the
subsequent open/unlink operations when applying the change.
| except Exception as e: | ||
| self.send_response(500) | ||
| self.send_header("Content-Type", "text/plain") | ||
| self.end_headers() | ||
| self.wfile.write(f"Error: {e}".encode()) | ||
| return |
There was a problem hiding this comment.
HIGH: Bare Exception catch violates the specific-exception guideline.
Per coding guidelines: "Do not catch bare Exception — catch specific exception types only." The try block can raise ImportError (if coverage isn't installed), OSError (file I/O), and coverage.CoverageException (report generation failures). Catching them explicitly also makes the error response more informative.
🐛 Proposed fix
After moving import coverage to module level (per the earlier comment):
- except Exception as e:
+ except (ImportError, OSError) as e:If coverage.CoverageException is accessible after the guarded import, include it as well:
- except Exception as e:
+ except (ImportError, OSError, _coverage_module.CoverageException) as e:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception as e: | |
| self.send_response(500) | |
| self.send_header("Content-Type", "text/plain") | |
| self.end_headers() | |
| self.wfile.write(f"Error: {e}".encode()) | |
| return | |
| except (ImportError, OSError) as e: | |
| self.send_response(500) | |
| self.send_header("Content-Type", "text/plain") | |
| self.end_headers() | |
| self.wfile.write(f"Error: {e}".encode()) | |
| return |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 31-31: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@coverport/coverage_server.py` around lines 31 - 36, Replace the bare "except
Exception as e" in the report-generation try block with explicit exception
handlers: catch ImportError (if coverage import fails), OSError (file I/O
problems) and, if coverage was imported at module level,
coverage.CoverageException (report generation errors); update the response
logging and message in each handler to include the specific exception details
and keep the same HTTP 500 response flow (this change touches the except block
that writes to self.wfile and calls
self.send_response/self.send_header/self.end_headers).
| def log_message(self, format, *args): | ||
| pass # Suppress request logs |
There was a problem hiding this comment.
MEDIUM: format parameter shadows the Python built-in.
Ruff A002 correctly flags this. The override of BaseHTTPRequestHandler.log_message need not keep the original parameter name; rename to fmt (or any non-shadowing name) with a matching type annotation:
♻️ Proposed fix
- def log_message(self, format, *args):
- pass # Suppress request logs
+ def log_message(self, fmt: str, *args: object) -> None:
+ pass # Suppress request logs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def log_message(self, format, *args): | |
| pass # Suppress request logs | |
| def log_message(self, fmt: str, *args: object) -> None: | |
| pass # Suppress request logs |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 42-42: Function argument format is shadowing a Python builtin
(A002)
[warning] 42-42: Missing type annotation for *args
(ANN002)
🤖 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 `@coverport/coverage_server.py` around lines 42 - 43, The log_message method
overrides BaseHTTPRequestHandler's method but uses a parameter name 'format'
that shadows the built-in Python function format. Rename the 'format' parameter
to 'fmt' (or another non-shadowing name) in the method signature along with
adding a matching type annotation, and keep the method body as is to suppress
request logs.
| # CoverPort runtime coverage instrumentation | ||
| COPY coverport/ /app/coverport/ | ||
| RUN pip install --no-cache-dir coverage>=7.0 | ||
| ENV COVERAGE_PROCESS_START="/app/.coveragerc" | ||
| ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" | ||
| ENV COVERAGE_PORT="53700" |
There was a problem hiding this comment.
CRITICAL: COVERAGE_PROCESS_START points to a path that does not exist in the image.
COPY coverport/ /app/coverport/ only places coverport/ under /app/. The .coveragerc file is copied at line 53 via COPY --from=builder ${TEST_DIR}/ ${TEST_DIR}/, landing at /openshift-virtualization-tests/.coveragerc. Setting COVERAGE_PROCESS_START="/app/.coveragerc" points coverage at a nonexistent file, so the project configuration (branch coverage, omit patterns, LCOV output, etc.) is completely ignored at runtime.
🐛 Proposed fix
-ENV COVERAGE_PROCESS_START="/app/.coveragerc"
+ENV COVERAGE_PROCESS_START="${TEST_DIR}/.coveragerc"ARG TEST_DIR is already declared in this stage and is available for ENV substitution.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # CoverPort runtime coverage instrumentation | |
| COPY coverport/ /app/coverport/ | |
| RUN pip install --no-cache-dir coverage>=7.0 | |
| ENV COVERAGE_PROCESS_START="/app/.coveragerc" | |
| ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" | |
| ENV COVERAGE_PORT="53700" | |
| # CoverPort runtime coverage instrumentation | |
| COPY coverport/ /app/coverport/ | |
| RUN pip install --no-cache-dir coverage>=7.0 | |
| ENV COVERAGE_PROCESS_START="${TEST_DIR}/.coveragerc" | |
| ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" | |
| ENV COVERAGE_PORT="53700" |
🧰 Tools
🪛 Hadolint (2.14.0)
[warning] 70-70: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>
(DL3013)
🤖 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 `@Dockerfile` around lines 68 - 73, The COVERAGE_PROCESS_START env var is
pointing to /app/.coveragerc which doesn’t exist; update the Dockerfile to set
COVERAGE_PROCESS_START to the actual copied location using the existing ARG
TEST_DIR (which contains the path where .coveragerc was copied by COPY
--from=builder ${TEST_DIR}/ ${TEST_DIR}/). Find the COVERAGE_PROCESS_START
assignment and change it to reference TEST_DIR (e.g., use ENV
COVERAGE_PROCESS_START="${TEST_DIR}/.coveragerc") so coverage uses the real
.coveragerc at runtime.
|
|
||
| # CoverPort runtime coverage instrumentation | ||
| COPY coverport/ /app/coverport/ | ||
| RUN pip install --no-cache-dir coverage>=7.0 |
There was a problem hiding this comment.
HIGH: pip install bypasses the uv-managed virtualenv used by uv run pytest.
All other package installs in this stage (lines 60–63) use uv pip install, which installs into the project virtualenv. Plain pip install installs to the system Python. When uv run pytest activates the uv venv, coverage installed via bare pip will not be importable — meaning coverport/sitecustomize.py's import coverage will fail silently and coverage collection won't happen.
🔧 Proposed fix
-RUN pip install --no-cache-dir coverage>=7.0
+RUN uv pip install coverage>=7.0Also consider pinning to a specific version (e.g., coverage==7.6.1) to comply with Hadolint DL3013 and ensure reproducible builds.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN pip install --no-cache-dir coverage>=7.0 | |
| RUN uv pip install coverage>=7.0 |
🧰 Tools
🪛 Hadolint (2.14.0)
[warning] 70-70: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>
(DL3013)
🤖 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 `@Dockerfile` at line 70, The RUN pip install line installs coverage into the
system Python instead of the project's uv virtualenv; change it to use the
project's uv-managed installer (use the same command pattern as the other
installs, e.g., "uv pip install ...") so coverage is installed into the uv venv
used by uv run pytest, and pin a specific version like coverage==7.6.1 to make
the build reproducible and satisfy Hadolint DL3013; update the RUN invocation
that currently reads "RUN pip install --no-cache-dir coverage>=7.0" accordingly.
| COPY coverport/ /app/coverport/ | ||
| RUN pip install --no-cache-dir coverage>=7.0 | ||
| ENV COVERAGE_PROCESS_START="/app/.coveragerc" | ||
| ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" |
There was a problem hiding this comment.
MEDIUM: Trailing colon in PYTHONPATH adds the current working directory to sys.path.
PYTHONPATH is not set anywhere in this Dockerfile or the Fedora base image. ${PYTHONPATH} expands to an empty string, resulting in PYTHONPATH="/app/coverport:". The trailing : (empty component) resolves to . at Python runtime — effectively inserting the CWD (/openshift-virtualization-tests) into sys.path, which could cause unexpected module shadowing.
🔧 Proposed fix
-ENV PYTHONPATH="/app/coverport:${PYTHONPATH}"
+ENV PYTHONPATH="/app/coverport"No other PYTHONPATH is set in this stage, so a direct assignment is correct.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ENV PYTHONPATH="/app/coverport:${PYTHONPATH}" | |
| ENV PYTHONPATH="/app/coverport" |
🤖 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 `@Dockerfile` at line 72, The ENV line sets PYTHONPATH with a trailing colon
which expands to an empty component and adds the CWD to sys.path; change the ENV
assignment for PYTHONPATH (the ENV PYTHONPATH="/app/coverport:${PYTHONPATH}"
line) to a direct assignment without the expansion so it becomes
PYTHONPATH="/app/coverport" to avoid inserting the current working directory
into Python's sys.path.
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Closing — this was a QualityFlow coverage onboarding test PR. Apologies for the noise. |
3 participants
Automated coverage onboarding for
RedHatQE/openshift-virtualization-testsvia QualityFlow bulk import.See the QualityFlow dashboard for details.
Summary by CodeRabbit
Tests
Chores