Skip to content

[io] Add gtest-based cpp11Containers unordered_map test - #23021

Open
vepadulano wants to merge 1 commit into
root-project:masterfrom
vepadulano:cpp11Containers-unorderedmap-gtest
Open

[io] Add gtest-based cpp11Containers unordered_map test#23021
vepadulano wants to merge 1 commit into
root-project:masterfrom
vepadulano:cpp11Containers-unorderedmap-gtest

Conversation

@vepadulano

Copy link
Copy Markdown
Member

This is a re-write of the cpp11Containers-unorderedMap test as a compiled C++ program based on gtest. Given the recent intermittent CI failures, the new test could help identify in a more fine-grained way the root cause. It is an addition rather than a substitution because there is no guarantee that the cause of the CI failure is in the test itself.

@vepadulano vepadulano self-assigned this Aug 5, 2026
@vepadulano
vepadulano force-pushed the cpp11Containers-unorderedmap-gtest branch 6 times, most recently from 8060e9c to ebcd9f7 Compare August 5, 2026 13:37
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 16h 48m 39s ⏱️
 3 858 tests  3 857 ✅ 0 💤 1 ❌
74 830 runs  74 828 ✅ 1 💤 1 ❌

For more details on these failures, see this check.

Results for commit f4badb0.

♻️ This comment has been updated with latest results.

@vepadulano
vepadulano force-pushed the cpp11Containers-unorderedmap-gtest branch 3 times, most recently from 8193652 to 4a52dd2 Compare August 6, 2026 09:46
This is a re-write of the cpp11Containers-unorderedMap test as a compiled C++
program based on gtest. Given the recent intermittent CI failures, the new test
could help identify in a more fine-grained way the root cause. It is an addition
rather than a substitution because there is no guarantee that the cause of the
CI failure is in the test itself.
@vepadulano
vepadulano force-pushed the cpp11Containers-unorderedmap-gtest branch from 4a52dd2 to f4badb0 Compare August 6, 2026 13:29
@vepadulano vepadulano closed this Aug 7, 2026
@vepadulano vepadulano reopened this Aug 7, 2026
@vepadulano
vepadulano requested a review from pcanal August 7, 2026 09:42
@vepadulano
vepadulano marked this pull request as ready for review August 7, 2026 09:42
@vepadulano

Copy link
Copy Markdown
Member Author

I've seen the unorderedMap macro-based test fail already a couple of times in this PR and at the same time the equivalent gtest-based test introduced by these changes was not failing. I propose we merge this extra test and see in the long-term if it fails over multiple CI runs, and if not we could just remove the macro-based one.

auto copy = obj;
fillObj(copy, gaus, uniform, rng);

// Write objects to file row-wise, i.e. as a whole object

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Odd semantic. We usually use row-wise for case of TTree/RNTuple where the object is stored in a single (unsplit) branch/field.

Suggested change
// Write objects to file row-wise, i.e. as a whole object
// Write objects directly in the TFile, as a whole object

In addition the content of the collection itself can be stored is member-wise or object-wise (the former is writing for each data member for each object and the later `for each object for each data member)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If there is interest in test the internal member-wise vs objects wise storing of a collection, this can be controlled by call TClass::SetCanSplit( bool ) for the contained class (eg. std::pair<K,V>) before the I/O operation.

// We want to match associative containers of the `key:value` type. The variadic
// template parameter list is used to help all compilers match this overload
template <typename KeyType, typename ValueType, template <typename... Args> typename Cont, typename... Args>
void check_eq(const Cont<KeyType, ValueType, Args...> &a, const Cont<KeyType, ValueType, Args...> &b)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should there be a different test for ordered and un-ordered containers? (i.e. to (double) check the ordering is correct for the ordered containers?)

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