Skip to content
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

Added the TestProcessor system #23194

Merged
merged 20 commits into from May 14, 2018
Merged

Added the TestProcessor system #23194

merged 20 commits into from May 14, 2018

Conversation

Dr15Jones
Copy link
Contributor

The TestProcessor system allows one to write unit tests for individual CMSSW modules.

Initial commit of the TestProcessor testing infrastructure. This version
can load a module and run it.
The very last data product in the Principal can now also use the SingleChoiceNoProcessProductResolver optimization if appropriate.
This should have been available when SingleChoiceNoProcessProductResolver was first added, but was an oversight.
TestProcessorConfig needs to be able to construct a EDPutTokenT.
Extended TestProcessor to allow testing of EDFilters. One can use the edm::test::Event to get access to the filters result.
Can register additional processes to the test and provide data from those processes.
Added an option to have the module check the expected value for each Run. If the value does not match, an exception will be thrown.
In addition to specifying a module to test, one can also use a series of needed EDProducers via a Task to help with the testing.
Tests can now specify EventSetup data products to be accessible by the modules.
Also fixed formatting.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23194/4642

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23194/4642/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23194/4642/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

@cmsbuild
Copy link
Contributor

-1

Tested at: 37e4cbc

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23194/27923/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests ClangBuild

  • Unit Tests:

I found errors in the following unit tests:

---> test unitTestsGroup_4 had ERRORS
---> test unitTestsGroup_1 had ERRORS

  • Clang:

I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 48 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

In file included from /cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/boost/1.63.0-omkpbe2/include/boost/python.hpp:11:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/boost/1.63.0-omkpbe2/include/boost/python/args.hpp:8:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/boost/1.63.0-omkpbe2/include/boost/python/detail/prefix.hpp:13:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/boost/1.63.0-omkpbe2/include/boost/python/detail/wrap_python.hpp:151:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/python/2.7.11-omkpbe2/include/python2.7/Python.h:85:
/cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/python/2.7.11-omkpbe2/include/python2.7/unicodeobject.h:534:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register PyObject *obj,     /* Object */
    ^~~~~~~~~
/cvmfs/cms-ib.cern.ch/nweek-02523/slc6_amd64_gcc630/external/python/2.7.11-omkpbe2/include/python2.7/unicodeobject.h:553:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register PyObject *obj      /* Object */
    ^~~~~~~~~


@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23194/27923/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 30
  • DQMHistoTests: Total histograms compared: 2715235
  • DQMHistoTests: Total failures: 2241
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2712811
  • DQMHistoTests: Total skipped: 183
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 29 files compared)
  • Checked 124 log files, 14 edm output root files, 30 DQM output files

@Dr15Jones
Copy link
Contributor Author

+1

@Dr15Jones
Copy link
Contributor Author

@fabiocos @smuzaffar the unit test and clang compilation errors are false positives.

The unit test failures are the ones in the IB caused by the change to jemalloc which changed the allowed configuration parameters.

The clang compilation error stems from python2.7 headers not being C++17 compatible.

@fabiocos
Copy link
Contributor

@Dr15Jones for the unit tests ok, @smuzaffar is modifying the jemalloc tool file. Concerning the other issue, as far as I can see the problem is not caused by this PR, just a pre-existent issue that is put in evidence by it, as headers including BootPython.h are used. Still, should we consider this just as "normal"?

@Dr15Jones
Copy link
Contributor Author

@fabiocos I think we should make an issue about th python error but not keep this pull request from being integrated because of it.

@Dr15Jones
Copy link
Contributor Author

@slava77 @perrotta the reconstruction signature is only because I had to disambiguate a namespace in the file 'DataFormats/TrackReco/interface/HitPattern.h'

@perrotta
Copy link
Contributor

+1

  • For reco, it solves ambiguity in DataFormats/TrackReco/interface/HitPattern.h (technical)
  • Jenkins tests pass (excluding the unit test and the clang build discussed elsewhere in this thread)

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@Dr15Jones
Copy link
Contributor Author

@fabiocos it would be nice to get this into the release today so when I speak about it tomorrow someone could give it a try.

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 1b8e196 into cms-sw:master May 14, 2018
@Dr15Jones Dr15Jones deleted the testHarness branch May 15, 2018 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants