jamesoff
Unit test execution
In the git checkout 679353b0a0656bdbeed8293bb1a342f76f77417f, if I run the unit tests with pytest I get 23 failures (https://gist.github.com/cpina/e1daef2c0687f0a01baaa8d950f44eb4)
If I do checkout the version v1.12.0 I get 3 failures (https://gist.github.com/cpina/1d1fa39cd6a9784fce2791ea0dea1cd2).
Two questions:
- What the best way to run the tests?
- I am packaging v1.12.0 for Debian (latest released version) and skipping the three failing unit tests. If we could confirm that it's a failing unit test, not a failing code, it would be great!
Interesting, I don't get the multiple failures I see in your first run, which look mostly related to the format returned by the Alerter message builder not matching what the test is expecting. I've seen that before I think, I'll look in to it some more.
The three failures you get otherwise I can reproduce - they're caused by not having a html directory which the test really should create I guess. mkdir html before running the tests fixes it - the github workflow does it too.
Just pytest should be enough to run the tests correctly; the unit-test target in the makefile adds some options for coverage checking. I wouldn't necessarily rely on the integration tests from the makefile as they're not bulletproof.
Interesting, I don't get the multiple failures I see in your first run, which look mostly related to the format returned by the Alerter message builder not matching what the test is expecting. I've seen that before I think, I'll look in to it some more.
I cannot reproduce it at the moment. I wouldn't worry too much.
The three failures you get otherwise I can reproduce - they're caused by not having a html directory which the test really should create I guess. mkdir html before running the tests fixes it - the github workflow does it too.
I confirm that, if "html" exists, the tests pass.
It would be really great if the directory is created by simplemonitor's tests when needed (actually, perhaps even using tempfile.mkdtemp?). It sounds silly, but in the context of Debian packaging it's hard to create the directory correctly (because of how the tests are invoked with multiple python versions and the usual infrastructure of packaging, also the tests are run in two different environments at build time and integration time).
I've edited this comment...
One more question, related to tests.
Could you run the tests this way?:
$ TZ=GMT+12 pytest
For me, if I'm in simplemonitor v1.12.0:
carles@pinux:~$ date
dimecres, 14 de febrer de 2024, 08:36:12 GMT
carles@pinux:~$
Run just with pytest: all good: https://gist.github.com/cpina/1856ceb5af850c21badefea04d44f1ec
If I run with "TZ=GMT+12 pytest" I get failures: https://gist.github.com/cpina/8800b9e62af9bf2aee2adf4985d0e75f
This came because the default Debian pipeline use reprotest (https://pypi.org/project/reprotest/) to check if a package is reproducible (and the package build step runs tests).
I was having issues running some tests. I've just seen that if I'm in the tag v1.12.0: all good (as seen above), if I'm in the HEAD of develop branch: I get failures.
I definitely get failures when overriding the timezone, I'll take a look. Has a feeling of timezone formatting weirdness to it, possibly related to using freezegun for testing time-related things.
Nothing silly about that temp dir thing - it really should be doing that, but this was the first project I ever tried writing tests for so was feeling my way through it and learning as I went. I'll look at that too, or you're welcome to if you want :)
I definitely get failures when overriding the timezone, I'll take a look. Has a feeling of timezone formatting weirdness to it, possibly related to using freezegun for testing time-related things.
This might be related: https://github.com/spulec/freezegun/pull/526 I haven't looked into detail, it was suggested from while I was investigating / learning about reprotest: https://salsa.debian.org/reproducible-builds/reprotest/-/issues/11#note_464119
Regarding the html directory:
I'll look at that too
Thanks!
you're welcome to if you want :)
I'll let you know if I start. I am pretty busy, I'm trying to prioritise the battles :-) at the moment the Debian package system runs the tests with "-k not test_html" which is not blocking the packaging...
I've just tested the timezone issue again using. I have: 45fc32f5985f1aeb828e6159eabc1f223e7690cf , which includes https://github.com/jamesoff/simplemonitor/pull/1351/files and I see (execution with TZ=BST and TZ=UTC):
carles@pinux:[develop]~/git/simplemonitor$ date
dimarts, 6 d’agost de 2024, 23:13:50 BST
carles@pinux:[develop]~/git/simplemonitor$ pytest
==================================== test session starts =====================================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0+repack
PySide2 5.15.8 -- Qt runtime 5.15.8 -- Qt compiled 5.15.8
rootdir: /home/carles/git/simplemonitor
plugins: mock-3.12.0, socket-0.6.0, anyio-3.6.2, cov-4.0.0, qt-4.2.0+repack, requests-mock-1.9.3, xvfb-2.0.0
collected 161 items
tests/test_alerter.py .............................FFFF.F............................. [ 39%]
....... [ 44%]
tests/test_envconfig.py .. [ 45%]
tests/test_fortysixelks.py . [ 45%]
tests/test_host.py .......... [ 52%]
tests/test_htmllogger.py . [ 52%]
tests/test_logger.py .......................... [ 68%]
tests/test_main.py ........... [ 75%]
tests/test_monitor.py ............... [ 85%]
tests/test_network_new.py .. [ 86%]
tests/test_service.py ... [ 88%]
tests/test_util.py ................... [100%]
========================================== FAILURES ==========================================
___________________________ TestAlerter.test_should_alert_catchup ____________________________
self = <test_alerter.TestAlerter testMethod=test_should_alert_catchup>
def test_should_alert_catchup(self):
config = {
"delay": 1,
"times_type": "only",
"time_lower": "10:00",
"time_upper": "11:00",
}
a = alerter.Alerter(config)
a.support_catchup = True
m = monitor.MonitorFail("fail", {})
m.run_test()
with freeze_time("2020-03-10 09:00", tz_offset=self.utcoffset):
> self.assertEqual(a.should_alert(m), alerter.AlertType.NONE)
E AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
tests/test_alerter.py:323: AssertionError
____________________________ TestAlerter.test_should_alert_limit _____________________________
self = <test_alerter.TestAlerter testMethod=test_should_alert_limit>
def test_should_alert_limit(self):
config = {
"times_type": "only",
"time_lower": "10:00",
"time_upper": "11:00",
"limit": 2,
}
a = alerter.Alerter(config)
m = monitor.MonitorFail("fail", {})
m.run_test()
with freeze_time("2020-03-10 10:30", tz_offset=self.utcoffset):
self.assertEqual(a.should_alert(m), alerter.AlertType.NONE)
self.assertEqual(a._ooh_failures, [])
m.run_test()
> self.assertEqual(a.should_alert(m), alerter.AlertType.FAILURE)
E AssertionError: <AlertType.NONE: 'none'> != <AlertType.FAILURE: 'failure'>
tests/test_alerter.py:280: AssertionError
__________________________ TestAlerter.test_should_alert_limit_ooh ___________________________
self = <test_alerter.TestAlerter testMethod=test_should_alert_limit_ooh>
def test_should_alert_limit_ooh(self):
config = {
"times_type": "only",
"time_lower": "10:00",
"time_upper": "11:00",
"limit": 2,
}
a = alerter.Alerter(config)
m = monitor.MonitorFail("fail", {})
m.run_test()
with freeze_time("2020-03-10 09:00", tz_offset=self.utcoffset):
self.assertEqual(a.should_alert(m), alerter.AlertType.NONE)
self.assertEqual(a._ooh_failures, [])
a = alerter.Alerter(config)
m.run_test()
> self.assertEqual(a.should_alert(m), alerter.AlertType.NONE)
E AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
tests/test_alerter.py:303: AssertionError
__________________________ TestAlerter.test_should_alert_no_catchup __________________________
self = <test_alerter.TestAlerter testMethod=test_should_alert_no_catchup>
def test_should_alert_no_catchup(self):
config = {
"delay": 1,
"times_type": "only",
"time_lower": "10:00",
"time_upper": "11:00",
}
a = alerter.Alerter(config)
m = monitor.MonitorFail("fail", {})
m.run_test()
with freeze_time("2020-03-10 09:00", tz_offset=self.utcoffset):
> self.assertEqual(a.should_alert(m), alerter.AlertType.NONE)
E AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
tests/test_alerter.py:341: AssertionError
_____________________________ TestAlerter.test_should_alert_ooh ______________________________
self = <test_alerter.TestAlerter testMethod=test_should_alert_ooh>
def test_should_alert_ooh(self):
config = {"times_type": "only", "time_lower": "10:00", "time_upper": "11:00"}
a = alerter.Alerter(config)
m = monitor.MonitorFail("fail", {})
m.run_test()
with freeze_time("2020-03-10 10:30", tz_offset=self.utcoffset):
> self.assertEqual(a.should_alert(m), alerter.AlertType.FAILURE)
E AssertionError: <AlertType.NONE: 'none'> != <AlertType.FAILURE: 'failure'>
tests/test_alerter.py:262: AssertionError
====================================== warnings summary ======================================
../../../../usr/lib/python3/dist-packages/shiboken2/files.dir/shibokensupport/feature.py:139
/usr/lib/python3/dist-packages/shiboken2/files.dir/shibokensupport/feature.py:139: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
return original_import(name, *args, **kwargs)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================== short test summary info ===================================
FAILED tests/test_alerter.py::TestAlerter::test_should_alert_catchup - AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
FAILED tests/test_alerter.py::TestAlerter::test_should_alert_limit - AssertionError: <AlertType.NONE: 'none'> != <AlertType.FAILURE: 'failure'>
FAILED tests/test_alerter.py::TestAlerter::test_should_alert_limit_ooh - AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
FAILED tests/test_alerter.py::TestAlerter::test_should_alert_no_catchup - AssertionError: <AlertType.FAILURE: 'failure'> != <AlertType.NONE: 'none'>
FAILED tests/test_alerter.py::TestAlerter::test_should_alert_ooh - AssertionError: <AlertType.NONE: 'none'> != <AlertType.FAILURE: 'failure'>
========================== 5 failed, 156 passed, 1 warning in 3.98s ==========================
carles@pinux:[develop]~/git/simplemonitor$ TZ=UTC pytest
==================================== test session starts =====================================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0+repack
PySide2 5.15.8 -- Qt runtime 5.15.8 -- Qt compiled 5.15.8
rootdir: /home/carles/git/simplemonitor
plugins: mock-3.12.0, socket-0.6.0, anyio-3.6.2, cov-4.0.0, qt-4.2.0+repack, requests-mock-1.9.3, xvfb-2.0.0
collected 161 items
tests/test_alerter.py ................................................................ [ 39%]
....... [ 44%]
tests/test_envconfig.py .. [ 45%]
tests/test_fortysixelks.py . [ 45%]
tests/test_host.py .......... [ 52%]
tests/test_htmllogger.py . [ 52%]
tests/test_logger.py .......................... [ 68%]
tests/test_main.py ........... [ 75%]
tests/test_monitor.py ............... [ 85%]
tests/test_network_new.py .. [ 86%]
tests/test_service.py ... [ 88%]
tests/test_util.py ................... [100%]
====================================== warnings summary ======================================
../../../../usr/lib/python3/dist-packages/shiboken2/files.dir/shibokensupport/feature.py:139
/usr/lib/python3/dist-packages/shiboken2/files.dir/shibokensupport/feature.py:139: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
return original_import(name, *args, **kwargs)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=============================== 161 passed, 1 warning in 3.92s ===============================
carles@pinux:[develop]~/git/simplemonitor$
Thanks for re-testing. I'll have another go at trying to get my head round these :)
I ran into a similar issue, before coming across this one. I have some progress towards a fix in #1598.
Thanks @wyardley for fixing this! Will make simplemonitor package reproducible and no need to skip test on building it.
@jamesoff : are you planning a simplemonitor release some time soon? Debian will enter freeze mode next months, in different stages (but until 15th April there shouldn't be any problem, maybe even May but unsure). I'm even considering backporting some recent fixes but I'd like to avoid it if possible.