Fix tests for Tools, fixes #1214

[dneise]

Fixes #1025

[dneise]

So first I make sure the test fails.

[codecov]

Codecov Report

Merging #1027 into master will increase coverage by 0.04%.
The diff coverage is 96.90%.

@@            Coverage Diff             @@
##           master    #1027      +/-   ##
==========================================
+ Coverage   87.00%   87.04%   +0.04%     
==========================================
  Files         192      192              
  Lines       12185    12192       +7     
==========================================
+ Hits        10601    10612      +11     
+ Misses       1584     1580       -4     

Impacted Files	Coverage Δ
ctapipe/core/container.py	74.76% <ø> (ø)
ctapipe/io/eventsource.py	95.23% <ø> (ø)
ctapipe/tools/dump_instrument.py	82.05% <ø> (ø)
ctapipe/tools/muon_reconstruction.py	73.33% <ø> (ø)
ctapipe/core/tool.py	87.91% <78.57%> (+2.10%)	⬆️
ctapipe/calib/camera/tests/test_calibrator.py	100.00% <100.00%> (ø)
ctapipe/calib/camera/tests/test_flatfield.py	92.50% <100.00%> (ø)
ctapipe/calib/camera/tests/test_pedestals.py	100.00% <100.00%> (ø)
ctapipe/containers.py	100.00% <100.00%> (ø)
ctapipe/core/__init__.py	100.00% <100.00%> (ø)
... and 22 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a7096...b47280a. Read the comment docs.

[kosack]

probably should rename this PR to show that it fixes all tools!

[watsonjj]

Can we also test tool.run(['--help']) for every tool? It can often raise some errors when traitlets have been changed, but not updated in the tool.

[watsonjj]

I have created #1034 to test for the help message of tools

[kosack]

this is mostly ready to merge, but there are a few minor conflicts

now commiter

[maxnoe]

From the travis log, it seems traitlets applications have an exit method that raises sys.exit which is called by default.

[maxnoe]

Ok, now I am satisfied with this.

• A new function run_tools catches the SystemExit and returns the status code

• A semantic change: the finish method is not called under error conditions anymore.
  The charge extractor used this for writing the file, even e.g. if an exception occured in start.
  I think it is also hioghly surprising, that on error some possibly complex code is started.

[maxnoe]

@kosack codacy complains that I use asserts in unit tests, do we really want to continue using that?

[kosack]

@kosack codacy complains that I use asserts in unit tests, do we really want to continue using that?

It has a lot of annoying things like that, but it is quite configurable. I just didn't find a good way to ignore unit tests. Basically it just runs prospector and pyflakes. We used to use another system, but it had even worse problems. Until CTAO provides one, let's just stick with it and try to update it's configuration.

https://support.codacy.com/hc/en-us/articles/115002130625-Codacy-Configuration-File

Would be nice to disable only some warnings in tests, however, not all checks.

Though given this: fossasia/query-server#332 it seems they will not fix it. Perhaps there is a better system out there.

[watsonjj]

@maxnoe Do you have a moment to fix the conflicts, so we can push to get this merged?

[maxnoe]

Ahyes, sorry I thought this was already merged

[maxnoe]

@kosack @watsonjj this should be done now