Skip to content

FORM Storage Tests#354

Merged
knoepfel merged 18 commits intomainfrom
feature/aolivier-form-storage-tests
Mar 2, 2026
Merged

FORM Storage Tests#354
knoepfel merged 18 commits intomainfrom
feature/aolivier-form-storage-tests

Conversation

@aolivier23
Copy link
Contributor

Add more unit tests for FORM. This battery of unit tests focuses on the storage layer of FORM. These tests are only applied to the TBranch/TTree family of containers for now, but they are designed to eventually be applied to other technology choices.

read a data product from disk with a type that doesn't match the type on
disk.
Association.  This models a physics algorithm in phlex writing two data
products through FORM.
@aolivier23
Copy link
Contributor Author

@phlexbot format

@github-actions
Copy link
Contributor

No automatic jsonnetfmt fixes were necessary.

@github-actions
Copy link
Contributor

No automatic markdownlint fixes were necessary.

@github-actions
Copy link
Contributor

No automatic cmake-format fixes were necessary.

@github-actions
Copy link
Contributor

Automatic clang-format fixes pushed (commit 4d27dc1).
⚠️ Note: Some issues may require manual review and fixing.

@aolivier23 aolivier23 changed the title Feature/aolivier form storage tests FORM Storage Tests Feb 23, 2026
@aolivier23
Copy link
Contributor Author

@phlexbot format

@github-actions
Copy link
Contributor

No automatic cmake-format fixes were necessary.

@github-actions
Copy link
Contributor

No automatic jsonnetfmt fixes were necessary.

@github-actions
Copy link
Contributor

Automatic clang-format fixes pushed (commit 0388f10).
⚠️ Note: Some issues may require manual review and fixing.

@github-actions
Copy link
Contributor

No automatic markdownlint fixes were necessary.

gemmeren
gemmeren previously approved these changes Feb 24, 2026
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
+ Coverage   85.36%   86.55%   +1.19%     
==========================================
  Files         122      122              
  Lines        2433     2433              
  Branches      389      389              
==========================================
+ Hits         2077     2106      +29     
+ Misses        233      208      -25     
+ Partials      123      119       -4     
Flag Coverage Δ
unittests 86.55% <ø> (+1.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8759264...09ea1af. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dangling pointers.  Thanks for catching that, Barnaliy!
wwuoneway
wwuoneway previously approved these changes Feb 24, 2026
Copy link
Contributor

@wwuoneway wwuoneway left a comment

Choose a reason for hiding this comment

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

thanks, LGTM

@aolivier23 aolivier23 dismissed stale reviews from wwuoneway and gemmeren via e241cd2 February 24, 2026 16:13
@aolivier23 aolivier23 requested a review from barnaliy February 24, 2026 16:33
@barnaliy
Copy link
Contributor

Thanks, @aolivier23, for all your work.

barnaliy
barnaliy previously approved these changes Feb 24, 2026
wwuoneway
wwuoneway previously approved these changes Feb 24, 2026
Copy link
Contributor

@wwuoneway wwuoneway left a comment

Choose a reason for hiding this comment

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

thanks, LGTM

@aolivier23 aolivier23 marked this pull request as ready for review February 24, 2026 17:49
propagates automatically: the root_storage library itself.  This
eliminates a duplicated USE_ROOT_STORAGE flag in
test/form/CMakeLists.txt.
@aolivier23 aolivier23 dismissed stale reviews from wwuoneway and barnaliy via 163399e February 26, 2026 16:29
@aolivier23 aolivier23 requested a review from wwuoneway February 26, 2026 17:15
wwuoneway
wwuoneway previously approved these changes Feb 27, 2026
Copy link
Contributor

@wwuoneway wwuoneway left a comment

Choose a reason for hiding this comment

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

thanks, the changes look fine.

gemmeren
gemmeren previously approved these changes Feb 27, 2026
Copy link
Contributor

@gemmeren gemmeren left a comment

Choose a reason for hiding this comment

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

Looks good, you may want to adjust the #include guards.

@aolivier23 aolivier23 dismissed stale reviews from gemmeren and wwuoneway via a586242 February 27, 2026 19:58
gemmeren
gemmeren previously approved these changes Feb 27, 2026
Copy link
Contributor

@gemmeren gemmeren left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Contributor

@gemmeren gemmeren left a comment

Choose a reason for hiding this comment

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

Removing asserts is fine by me. Thanks Andrew, Barnali

@barnaliy
Copy link
Contributor

barnaliy commented Mar 2, 2026

Although I’ve approved the PR, I noticed that the Code Coverage check is currently failing. I’m sure this will be taken care of shortly.

@aolivier23
Copy link
Contributor Author

Although I’ve approved the PR, I noticed that the Code Coverage check is currently failing. I’m sure this will be taken care of shortly.

Thanks Barnali, Kyle and I are taking a look at the test failure. We think it's unrelated to FORM. I will make sure tests pass before this merges.

@knoepfel knoepfel merged commit 0435987 into main Mar 2, 2026
61 of 62 checks passed
@knoepfel knoepfel deleted the feature/aolivier-form-storage-tests branch March 2, 2026 16:32
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.

6 participants