Skip to content

Conversation

@guitargeek
Copy link
Collaborator

@guitargeek guitargeek commented Dec 4, 2025

Many tests that were hardcoded in the YAML config of the CMSSW CI are also in the CMake tests, which are always run on all CI platforms.

While it makes sense to keep some test around also in the explicit YAML config so that the output can be inspected manually, this doesn't make sense anymore for tests that compare the output against a reference file, as no manual inspection of the output is required.

This commit removes such tests from the CMSSW CI.

Summary by CodeRabbit

  • Chores
    • Streamlined CI workflow by removing select test steps while retaining core validation tests and general analysis workflows. Pipeline focus refined to essential testing procedures.

✏️ Tip: You can customize this high-level summary in your review settings.

Many tests that were hardcoded in the YAML config of the CMSSW CI are
also in the CMake tests, which are always run on all CI platforms.

While it makes sense to keep some test around also in the explicit YAML
config so that the output can be inspected manually, this doesn't make
sense anymore for tests that compare the output against a reference
file, as no manual inspection of the output is required.

This commit removes such tests from the CMSSW CI.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

The pull request removes multiple template analysis and counting test steps from the CVMFS CI workflow, including parametric analyses, CMSHistFunc variants, and CMSHistSum configurations, while preserving core testing infrastructure and broader analysis capabilities.

Changes

Cohort / File(s) Summary
CI workflow step removals
.github/workflows/cvmfs-ci.yml
Removed multiple run-in-cvmfs test steps: counting datacard, parametric analysis, template analyses (CMSHistFunc, CMSHistFunc shapeN, CMSHistSum variants), CMSHistSum with channel masks, and CMSHistSum with shapeN and channel masks variants. Retained counting datacard Fixed Point, RooMultiPdf, RooParametricHist, RooHistPdf steps, and core Run Tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify that removed steps are intentionally disabled and not critical to active development
  • Confirm retained steps cover essential functionality (RooMultiPdf, RooParametricHist, RooHistPdf, Fixed Point)
  • Check for any hardcoded dependencies elsewhere that may reference the removed step names

Possibly related PRs

Poem

🐰 Steps are trimmed from the workflow tree,
Template tests made history,
CMSHistSum bids farewell,
Core tests remain to serve us well!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[ci] Remove redundant tests from the CMSSW CI' directly and clearly summarizes the main change: removing redundant CI workflow tests, which matches the pull request objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61aa0b6 and 282bba8.

📒 Files selected for processing (1)
  • .github/workflows/cvmfs-ci.yml (0 hunks)
💤 Files with no reviewable changes (1)
  • .github/workflows/cvmfs-ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Compile (py3.12, root6.34.4)
  • GitHub Check: Compile (py3.10, root6.32.2)
  • GitHub Check: Compile (py3.10, root6.26.4)
  • GitHub Check: CMSSW_11_3_4 - ROOT 6.22.09
  • GitHub Check: CMSSW_14_0_0_pre1 - ROOT 6.26.11
  • GitHub Check: LCG_106 - ROOT 6.32.02
  • GitHub Check: dev3/latest - ROOT LCG master
  • GitHub Check: LCG_108 - ROOT 6.36.02
  • GitHub Check: CMSSW_14_1_0_pre4 - ROOT 6.30.07
  • GitHub Check: LCG_102 - ROOT 6.26.04

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anigamova anigamova merged commit a77c0a0 into cms-analysis:main Dec 4, 2025
14 checks passed
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 20.69%. Comparing base (61aa0b6) to head (282bba8).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1189   +/-   ##
=======================================
  Coverage   20.69%   20.69%           
=======================================
  Files         195      195           
  Lines       26154    26154           
  Branches     3923     3923           
=======================================
  Hits         5412     5412           
  Misses      20742    20742           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guitargeek guitargeek deleted the redundant_tests branch December 4, 2025 10:51
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.

2 participants