-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[ntuple] Add Merger tests for Streamer fields #19484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ntuple] Add Merger tests for Streamer fields #19484
Conversation
Test Results 21 files 21 suites 3d 3h 55m 54s ⏱️ For more details on these failures, see this check. Results for commit 0b91676. ♻️ This comment has been updated with latest results. |
027e773
to
bfb6c8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Also change the failure in GenerateZeroPages from a R__LOG_FATAL to a ROOT::Result failure
Also add [[nodiscard]] to RResultBase::ForwardError to prevent erroneous use of R__FORWARD_ERROR
bfb6c8f
to
0b91676
Compare
if (fIsInitialized) | ||
for (const auto &field : changeset.fAddedFields) | ||
if (field->GetStructure() == ENTupleStructure::kStreamer) | ||
throw ROOT::RException(R__FAIL("a Model cannot be extended with Streamer fields")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change should probably also be tested outside the merger, in tree/ntuple/test/ntuple_modelext.cxx
, as it's a general restriction...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also change the failure in GenerateZeroPages from a R__LOG_FATAL to a ROOT::Result failure (we don't really need to abort the whole process if merging fails)
Checklist: