Skip to content

Conversation

@greenc-FNAL
Copy link
Contributor

Refactors the coverage.sh script to align its behavior, particularly for the Clang toolchain, with the CI coverage workflow.

Key changes:

  • Sets the LLVM_PROFILE_FILE environment variable during the test phase for Clang to ensure correct collection of raw profile data, mirroring the CI setup.
  • Changes the default preset from coverage-gcc to coverage-clang to match the CI standard.
  • Adds HTML report generation for the Clang toolchain, bringing it to feature parity with the GCC preset within the script.
  • Simplifies the logic for the Clang path by removing irrelevant and incorrect GCC-specific staleness checks.
  • Improves the argument parsing to be more robust and reliable.

Refactors the coverage.sh script to align its behavior with the CI workflow.

  • Implements support for uploading LLVM/Clang coverage reports to Codecov.
  • Improves the help text to clarify the differences between the GCC and Clang workflows.
  • Adds validation for the --preset flag.
  • Changes the exit code to 1 when no commands are provided.
  • Improves comments and file URL generation.

Refactors the `coverage.sh` script to align its behavior, particularly for the Clang toolchain, with the CI coverage workflow.

Key changes:
- Sets the `LLVM_PROFILE_FILE` environment variable during the test phase for Clang to ensure correct collection of raw profile data, mirroring the CI setup.
- Changes the default preset from `coverage-gcc` to `coverage-clang` to match the CI standard.
- Adds HTML report generation for the Clang toolchain, bringing it to feature parity with the GCC preset within the script.
- Simplifies the logic for the Clang path by removing irrelevant and incorrect GCC-specific staleness checks.
- Improves the argument parsing to be more robust and reliable.

Refactors the coverage.sh script to align its behavior with the CI workflow.

- Implements support for uploading LLVM/Clang coverage reports to Codecov.
- Improves the help text to clarify the differences between the GCC and Clang workflows.
- Adds validation for the --preset flag.
- Changes the exit code to 1 when no commands are provided.
- Improves comments and file URL generation.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the coverage.sh script to align it with the CI coverage workflow, with a focus on improving the Clang toolchain support and making it the default. The changes enhance consistency between local development and CI environments, improve argument parsing robustness, and add feature parity for HTML report generation across both GCC and Clang presets.

Key changes:

  • Sets Clang (coverage-clang) as the default preset to match CI workflow, with proper LLVM_PROFILE_FILE environment variable handling during test execution
  • Adds HTML report generation support for Clang preset, bringing it to feature parity with GCC
  • Refactors argument parsing to validate preset values and properly handle the case where options are provided without commands

Comment on lines +214 to +215
# GCC-specific staleness check for .gcno files
elif [[ "$COVERAGE_PRESET" == "coverage-gcc" ]]; then
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The condition has been inverted from "!= coverage-clang" to "== coverage-gcc", which changes the logic. The original code skipped staleness checks for Clang and performed them for all other presets (including gcc). The new code only performs staleness checks for GCC specifically. This means if someone uses a different preset name in the future, staleness checks won't run. While this may be intentional, consider documenting this assumption or using a more defensive check like "!= coverage-clang" to ensure staleness checks run for any non-Clang preset.

Suggested change
# GCC-specific staleness check for .gcno files
elif [[ "$COVERAGE_PRESET" == "coverage-gcc" ]]; then
# For non-Clang coverage presets, perform staleness checks for .gcno files
elif [[ "$COVERAGE_PRESET" != "coverage-clang" ]]; then

Copilot uses AI. Check for mistakes.
error "Please install lcov: 'sudo apt-get install lcov' or 'brew install lcov'"
exit 1
fi

Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The genhtml command uses several --ignore-errors flags (mismatch, inconsistent, negative, empty) which silently suppress potential data quality issues. While this may be necessary for practical reasons, consider logging a warning when these errors are ignored, or documenting why these specific errors are expected and can be safely ignored.

Suggested change
# NOTE: We intentionally ignore certain genhtml data inconsistencies to keep
# HTML report generation robust even when coverage data is imperfect (for example,
# due to partial builds or toolchain differences). These conditions are still
# worth investigating if they appear frequently.
log "WARNING: genhtml will ignore coverage data issues (mismatch,inconsistent,negative,empty) when generating the HTML report."

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant