Skip to content

feat: display CodeQL MISRA findings on nightly quality dashboard#720

Draft
Komal362000 wants to merge 2 commits into
eclipse-score:mainfrom
Komal362000:codeql-pr-clean-708
Draft

feat: display CodeQL MISRA findings on nightly quality dashboard#720
Komal362000 wants to merge 2 commits into
eclipse-score:mainfrom
Komal362000:codeql-pr-clean-708

Conversation

@Komal362000

Copy link
Copy Markdown
Contributor

Enable the nightly quality dashboard to surface CodeQL MISRA compliance reports generated in CI, and

Enable the nightly quality dashboard to surface CodeQL MISRA compliance
reports generated in CI, and fix the CodeQL CI job so the reports are
produced and uploaded reliably.

CI fixes (.github/workflows/_codeql.yml, quality/static_analysis/codeql_lint.py):
- Resolve the CodeQL analysis_report binary path correctly.
- Trigger a bazel cquery fetch of @codeql_coding_standards before
  searching for the pack root, so the pack root discovery reliably
  finds cpp/common/src in the bazel external cache.
- Add the CodeQL bin directory to PATH so analysis_report can find its
  dependencies.
- Make MISRA report generation/upload non-fatal, so a single report
  failure doesn't fail the whole workflow.

Dashboard wiring (.github/workflows/nightly_quality.yml,
bazel/rules/generate_quality_links.bzl, quality/dashboard/*,
quality/scripts/bundle_quality_reports.sh, quality/quality.md,
docs/sphinx/quality_reports.rst):
- Copy CodeQL MISRA reports into the quality output directory.
- Wire generated report links into the dashboard and docs.
exit 1
fi

"${CODEQL_BIN}" pack install "${PACK_ROOT}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want todo these steps in the CI. Are we sure that this is required? Because this would be a lot of logic outside of Bazel - which cannot be reproduced locally.

IMHO we should avoid this whole section.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, that section was working around something Bazel already does for us. I moved the patch and try to it runs identically whether invoked via bazel run locally or in CI.

Comment on lines -39 to -55
release_coverage_asset_url = ""
if docs_base_url.startswith("https://"):
url_without_scheme = docs_base_url[8:]
url_parts = url_without_scheme.split("/")
if len(url_parts) >= 2:
host = url_parts[0]
repo = url_parts[1]
if host.endswith(".github.io") and repo:
owner = host[:-len(".github.io")]
if owner.endswith("."):
owner = owner[:-1]
if owner:
release_coverage_asset_url = (
"https://github.com/" + owner + "/" + repo +
"/releases/download/" + docs_version + "/" +
repo + "_coverage_report_" + docs_version + ".zip"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes were required to get correct links for release builds - why are we removing it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right to flag this is a genuine mistake, not an intentional removal. The release_coverage_asset_url logic computes a GitHub release artifact download link for the coverage reports.

bazel run //quality/dashboard:generate_dashboard -- \\
--lcov /tmp/coverage_zip/extracted/artifacts/coverage_report.dat \\
--clang-tidy /tmp/clang_tidy/clang_tidy_findings.txt \\
--codeql-csv /tmp/codeql-results/codeql-nightly.csv \\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? If we do not use the attribute?

Comment on lines +50 to +53
In local Sphinx builds and in versioned release archives the link cells
above show the equivalent ``bazel run`` command or a link to the
``latest`` reports respectively. The deploy workflow controls this
automatically via the ``DOCS_VERSION`` build variable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to change this?

csv_path = f"{output_base}/{output_prefix}.csv"

# Run CodeQL analysis (generates SARIF)
print("\n Running CodeQL analysis...")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it to keep the CI logs clean and keep the PR small. If you think it's useful, I can add it back.


# Generate reports using CodeQL analysis_report tool
if analysis_report_path and os.path.exists(analysis_report_path):
print(" Generating MISRA C++ compliance reports...")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep this?

Comment on lines -101 to -103
# Remove existing reports directory if it exists
if os.path.exists(reports_output_dir):
shutil.rmtree(reports_output_dir)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the old reports before creating new ones. I first thought the new reports would replace the old ones, but they don't always do that. Some old files can still stay there and get included. Keeping this cleanup makes sure only the new reports are used.

@Komal362000
Komal362000 force-pushed the codeql-pr-clean-708 branch 2 times, most recently from 11cd660 to 0fc8b0a Compare July 17, 2026 10:16
- Add back print statements for analysis and report generation steps
- Restore directory cleanup to prevent stale MISRA reports from persisting
- Addresses reviewer feedback (@castler comments on PR eclipse-score#720)
Komal362000 added a commit to Komal362000/komal_communication that referenced this pull request Jul 17, 2026
…olve all PR eclipse-score#720 comments)

Resolve all 7 reviewer comments from @castler:

1. ✓ Move pack installation logic from bash to Python (runfiles API)
   - Removed 66-line bash install step from _codeql.yml
   - Implemented _find_pack_root() and _install_query_pack() in codeql_lint.py
   - Now reproducible locally and in CI (logic inside Bazel)

2. ✓ Restore release_coverage_asset_url logic in generate_quality_links.bzl
   - Required for release builds to compute GitHub artifact download URLs
   - Correctly points to pre-built coverage reports in releases

3. ✓ Restore visibility print statements in codeql_lint.py
   - 'Running CodeQL analysis...' provides step visibility
   - 'Generating MISRA C++ compliance reports...' aids progress tracking

4. ✓ Restore directory cleanup (shutil.rmtree) before report generation
   - Prevents old MISRA reports from persisting into new generation
   - Ensures only fresh reports are included in artifacts

5. ✓ Keep --codeql-csv flag in generate_dashboard.py
   - Required for dashboard CodeQL KPI metrics
   - Documented in quality.md with output location

6. ✓ Clarify docs wording in quality_reports.rst
   - Explicitly state link behavior for local/release/CI contexts
   - Documenting DOCS_VERSION control mechanism

7. ✓ Wire CodeQL MISRA reports into dashboard
   - Added 4 report link substitutions for integrity/deviations/compliance/recategorizations
   - Updated nightly workflow to download and bundle reports
   - Dashboard renders CodeQL metrics when data available
@Komal362000
Komal362000 force-pushed the codeql-pr-clean-708 branch 2 times, most recently from 677b8c9 to d2d4933 Compare July 17, 2026 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants