[HS3] [RF] Add HS3 conformance test suite integration by stalbrec · Pull Request #22567 · root-project/root

stalbrec · 2026-06-10T21:21:52Z

This Pull request:

Changes or fixes:

Adds a new hs3testsuite CMake option that clones the HS3TestSuite (https://github.com/Phmonski/HS3TestSuite) repository and runs the ROOT HS3 conformance checks, ensuring RooFit's JSON serialization stays compatible with the evolving schema.

Checklist:

tested changes locally
updated the docs (if necessary)

github-actions · 2026-06-10T23:59:17Z

Test Results

22 files 22 suites 3d 16h 48m 42s ⏱️
3 871 tests 3 870 ✅ 0 💤 1 ❌
77 372 runs 77 269 ✅ 102 💤 1 ❌

For more details on these failures, see this check.

Results for commit b544a19.

♻️ This comment has been updated with latest results.

guitargeek

Thank you for this awesome initiative! I really like the idea, but have some change requests about the implementation.

…cument it documents dependecies on network connection and other build options. Now throwing error if `pyroot` or `testing` option is set to OFF instead of implying them with hs3 option.

…ctory and pin to specific commit hash

stalbrec · 2026-06-16T13:33:34Z

@guitargeek Thank you very much for this great feedback!

I made all of the changes, except for moving the HS3TestSuite repo to the HS3 org - but I think we will do that soon.

@cburgard suggested that we should make the test suite backend agnostic, and move the backend logic to the respective backend src (i.e. for roofit in the roofit/hs3/test directory).

Should I add these changes to this PR or open another one?

guitargeek

Thanks for the updates! The git logic still needs to be a bit more safer, see my inline comment.

guitargeek · 2026-06-16T13:47:06Z

@cburgard suggested that we should make the test suite backend agnostic, and move the backend logic to the respective backend src (i.e. for roofit in the roofit/hs3/test directory).

I don't understand yet what you mean here, but in any case I'd do this in a separate PR. Let's get the CMake plumbing correct here for a first integration (even if the repo is still under Phmonski, we can move later), and then build on top of that. You're okay with that?

cburgard · 2026-06-16T14:13:53Z

@cburgard suggested that we should make the test suite backend agnostic, and move the backend logic to the respective backend src (i.e. for roofit in the roofit/hs3/test directory).

I don't understand yet what you mean here, but in any case I'd do this in a separate PR. Let's get the CMake plumbing correct here for a first integration (even if the repo is still under Phmonski, we can move later), and then build on top of that. You're okay with that?

Just to chime in here, the point I was making in the discussion is that right now, the repo includes a wrapper class called "RooFit Backend" . This is a bit unfortunate, as this requires that the repo knows about all the backends that should be supported by this suite. I would find it much more elegant if this backend could be moved here (to the ROOT github) such that the suite itself is completely agnostic about which backends might attempt to use it.

guitargeek · 2026-06-16T15:11:22Z

@cburgard suggested that we should make the test suite backend agnostic, and move the backend logic to the respective backend src (i.e. for roofit in the roofit/hs3/test directory).

I don't understand yet what you mean here, but in any case I'd do this in a separate PR. Let's get the CMake plumbing correct here for a first integration (even if the repo is still under Phmonski, we can move later), and then build on top of that. You're okay with that?

Just to chime in here, the point I was making in the discussion is that right now, the repo includes a wrapper class called "RooFit Backend" . This is a bit unfortunate, as this requires that the repo knows about all the backends that should be supported by this suite. I would find it much more elegant if this backend could be moved here (to the ROOT github) such that the suite itself is completely agnostic about which backends might attempt to use it.

Sure, I have nothing against moving this to ROOT! This test interface doesn't depend on the test suite itself as far as I can tell, so it's just pure PyROOT code anyway. @stalbrec, let me know if you have questions about where the Python code should go, but my preferred way to work is that you just try out something in a PR (unless you are completely stuck) and if it's not up to standard I'll let you know and we iterate 🙂

guitargeek · 2026-06-17T09:43:28Z

The jsonschema library was picked up in the latest builds of the ROOT CI containers: https://github.com/root-project/root-ci-images/actions/runs/27667025733

stalbrec · 2026-06-18T08:29:02Z

@guitargeek I think I have a good working version with the backend inside the root-repo. Also @Phmonski will migrate the TestSuite likely today to the hs3 org.

We propose to add these changes to this PR, if that's alright.

guitargeek · 2026-06-18T09:33:55Z

As you prefer! Whatever works for you. So I wait for that before doing the next round of review?

Phmonski · 2026-06-18T09:37:48Z

I migrated the test suite to https://github.com/hep-statistics-serialization-standard/HS3TestSuite

@guitargeek @stalbrec

Co-authored-by: Phmonski <simon.cello@web.de>

stalbrec · 2026-06-24T08:09:47Z

@guitargeek I made all the changes we wanted to. I'm not sure about the failing tests, but I am pretty sure they are unrelated to our changes.

guitargeek

Thanks again for the updates! Good that for you, everything works now.

@stalbrec, just one more change request for configuration stability 🙂

guitargeek · 2026-06-24T09:35:14Z

@guitargeek I made all the changes we wanted to. I'm not sure about the failing tests, but I am pretty sure they are unrelated to our changes.

Indeed, the failures are unrelated.

…ching

stalbrec · 2026-06-25T08:44:12Z

If it's alright we would like to change the GIT_TAG to an actual annotated tag still in this PR.

stalbrec · 2026-06-25T09:10:23Z

If it's alright we would like to change the GIT_TAG to an actual annotated tag still in this PR.

Ah, I just found out that the CMake documentation advises against doing that.

guitargeek

Very nice, thanks a lot for this addition and addressing all the review comments!

[HS3] [RF] Adding test to pull and run external HS3TestSuite

12e7039

stalbrec requested review from bellenot, dpiparo and guitargeek as code owners June 10, 2026 21:21

fix linting (import sorting)

b02d03b

dpiparo assigned guitargeek Jun 11, 2026

guitargeek requested changes Jun 11, 2026

View reviewed changes

guitargeek added in:RooFit new contributor in:Build System labels Jun 11, 2026

stalbrec added 2 commits June 16, 2026 14:40

[cmake] rename hs3testsuite option to test_roofit_hs3testsuite and do…

5b2bdb5

…cument it documents dependecies on network connection and other build options. Now throwing error if `pyroot` or `testing` option is set to OFF instead of implying them with hs3 option.

[CMake] Update hs3testsuite directory path to use current binary dire…

f6a0e5f

…ctory and pin to specific commit hash

stalbrec requested a review from hageboeck as a code owner June 16, 2026 13:26

guitargeek requested changes Jun 16, 2026

View reviewed changes

Comment thread roofit/hs3/test/CMakeLists.txt Outdated

stalbrec commented Jun 16, 2026

View reviewed changes

Comment thread roofit/hs3/test/CMakeLists.txt Outdated

[CMake] Use FetchContent for more robust HS3TestSuite download

0d462a0

stalbrec mentioned this pull request Jun 16, 2026

[ci] [pyroot] [rf] adding jsonschema dependency for hs3 validation #22635

Merged

2 tasks

stalbrec requested a review from guitargeek June 18, 2026 07:45

stalbrec and others added 3 commits June 19, 2026 11:14

adding roofit backend from hs3testsuite

09e7d24

Co-authored-by: Phmonski <simon.cello@web.de>

update HS3TestSuite repository URL to the new source

e2c4f09

making ruff happy

5786763

guitargeek requested changes Jun 24, 2026

View reviewed changes

Comment thread roofit/hs3/test/CMakeLists.txt Outdated

remove stale EXISTS guard and disable git prompt in HS3 testsuite fet…

b544a19

…ching

guitargeek approved these changes Jun 25, 2026

View reviewed changes

guitargeek merged commit c60cc22 into root-project:master Jun 25, 2026
31 of 33 checks passed

Uh oh!

Conversation

stalbrec commented Jun 10, 2026

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

github-actions Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stalbrec commented Jun 16, 2026

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guitargeek commented Jun 16, 2026

Uh oh!

cburgard commented Jun 16, 2026

Uh oh!

Uh oh!

guitargeek commented Jun 16, 2026

Uh oh!

guitargeek commented Jun 17, 2026

Uh oh!

stalbrec commented Jun 18, 2026

Uh oh!

guitargeek commented Jun 18, 2026

Uh oh!

Phmonski commented Jun 18, 2026

Uh oh!

stalbrec commented Jun 24, 2026

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guitargeek commented Jun 24, 2026

Uh oh!

stalbrec commented Jun 25, 2026

Uh oh!

stalbrec commented Jun 25, 2026

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jun 10, 2026 •

edited

Loading