-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add test for schema evolution of auto_ptr -> unique_ptr #48817
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
Conversation
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48817/45934
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
76ade85 to
15f2fea
Compare
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48817/45936
|
|
Pull request #48817 was updated. |
|
@cmsbuild, please test with cms-data/IOPool-Input#4 This test should fail, but I want it on the record. |
|
-1 Failed Tests: UnitTests The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see more details here: Unit TestsI found 1 errors in the following unit tests: ---> test TestIOPoolInputSchemaEvolution had ERRORS Comparison SummarySummary:
|
|
Already the splitlevel 0 test failed with |
|
Rebased to CMSSW_16_0_0_pre3, and (in a separate commit) removed the |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48817/47028
|
|
A new Pull Request was created by @makortel for master. It involves the following packages:
@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
test parameters: |
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 40KB to repository The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see more details here: DAS Queries: The DAS query tests failed, see the summary page for details. Comparison SummarySummary:
|
| <class name="edmtest::SchemaEvolutionPointerToUniquePtr" ClassVersion="3"> | ||
| <version ClassVersion="3" checksum="1545257825"/> | ||
| </class> | ||
| <class name="std::auto_ptr<edmtest::SchemaEvolutionContained>"/> |
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.
If std::auto_ptr actually goes away for some compiler, what will happen for this line?
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 line is in a commented out section that is needed only to create the input files of the test. If auto_ptr really goes away, we wouldn't be able to create/update the input files in those future CMSSW releases (or, we'd have to use old-enough CMSSW to create/update those files).
More generally the structure of these schema evolution tests made updating them to e.g. include new data types to be tested a bit complicated, so for RNTuple we might want to re-think about the test structure.
|
Comparison differences are related to #47071 |
|
+core |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @ftenchini, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
Needs to be merged together with cms-data/IOPool-Input#4 |
|
+1 |
PR description:
As a stepping stone to solve #43422 and #43923, this PR extends the schema evolution test to cover a case where an
std::auto_ptrclass member is evolved tostd::unique_ptr. With the first commit only and ROOT older than 6.36 the test will fail.The second commit applies the recipe #43923 (comment) to the test.
Needs the test file update from cms-data/IOPool-Input#4
Resolves cms-sw/framework-team#1451
PR validation:
In ROOT6.36 the schema evolution unit test succeeds.