-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[GDB] Added unit tests for gdb and running cmsTraceFunction #48714
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
|
please test with cms-sw/cmsdist#10009 |
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48714/45756 |
|
A new Pull Request was created by @smuzaffar for master. It involves the following packages:
@Dr15Jones, @makortel, @smuzaffar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
-1 Failed Tests: UnitTests Unit TestsI found 1 errors in the following unit tests: ---> test test-gdb-python had ERRORS Comparison SummarySummary:
|
|
please test with cms-sw/cmsdist#10009 |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48714/45758 |
|
Pull request #48714 was updated. @Dr15Jones, @makortel, @smuzaffar can you please check and sign again. |
|
+1 Size: This PR adds an extra 20KB 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: Comparison SummarySummary:
|
|
please test cms-sw/cmsdist#10009 change is now available in IBs |
|
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
|
| @@ -0,0 +1,9 @@ | |||
| #!/bin/bash -ex | |||
| g++ -o test-cmsTraceFunction-setenv $(dirname $0)/test-cmsTraceFunction-setenv.cpp | |||
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.
The purpose of $(dirname $0)/ is not immediately clear to me, given that the following line uses ./.
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.
As scram runs unit tests from $CMSSW_BASE/unittests/<test_name> directory, so $0 will be full path to test-cmsTraceFunction-setenv.sh and $(dirname $0) should be $CMSSW_BASE/src/Utilities/ReleaseScripts/test/gdb . Other option is to use ${ SCRAM_TEST_PATH}/gdb but then unit test can only run via scram b runtests
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.
Ah, the options specified the source file, not the target binary. Thanks, and never mind.
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.
A post-merge question: do we explicitly depend on g++ elsewhere in CMSSW already? I'm just wondering the hypothetical case we'd some day change our compiler.
|
+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, @mandrenguyen, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This PR proposes to add couple of unit tests to make sure a working GDB python
test-gdb-python: This unit test makes sure the python used by GDB is same as the one configured in cmssw. This will work once [gdb] build with relocatable python libdir cmsdist#10009 is mergedtest-cmsTraceFunction-setenv: It runcmsTraceFunction --startAfterFunction ScheduleItems::initMisc setenvto make sure gdp is properly breaking atsetenvafterScheduleItems::initMischas been called. We can extend it test other options ofcmsTraceFunctionFYI @makortel