Unit test execution #1330

cpina · 2024-01-23T23:33:33Z

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!

The text was updated successfully, but these errors were encountered:

jamesoff · 2024-02-04T15:16:27Z

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.

cpina · 2024-02-13T23:03:45Z

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).

cpina · 2024-02-14T08:32:00Z

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.

jamesoff · 2024-02-14T12:03:05Z

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 :)

cpina · 2024-02-14T21:34:51Z

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: spulec/freezegun#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...

cpina · 2024-08-06T22:15:44Z

I've just tested the timezone issue again using. I have: 45fc32f , 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$

jamesoff · 2024-08-11T15:29:08Z

Thanks for re-testing. I'll have another go at trying to get my head round these :)

Fixes jamesoff#1330 Some tests wouldn't pass locally if `TZ` was unset, or not set to to, e.g., UTC. One issue was the fallback to `"local"` if `TZ` was unset - this is a valid value internally within simplemonitor, but I don't think valid as a time zone. Also, having many tests use the local timezone creates some unpredictability in cases where there are hardcoded offsets, or where the test depends on the local timezone having a specific offset from UTC. Mock `TZ` as `MST` or `UTC` explicitly in the environment, and adjust existing tests based on offsets from that where needed. Unskip some skipped tests

wyardley · 2025-02-08T23:48:27Z

I ran into a similar issue, before coming across this one. I have some progress towards a fix in #1598.

Fixes jamesoff#1330 Some tests wouldn't pass locally if `TZ` was unset, or not set to to, e.g., UTC. One issue was the fallback to `"local"` if `TZ` was unset - this is a valid value internally within simplemonitor, but I don't think valid as a time zone. Also, having many tests use the local timezone creates some unpredictability in cases where there are hardcoded offsets, or where the test depends on the local timezone having a specific offset from UTC. Mock `TZ` as `UTC` or `MST` explicitly, and adjust existing tests based on offsets from that where needed. Fix / unskip some skipped tests

cpina · 2025-02-27T10:43:05Z

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.

jamesoff · 2025-02-27T12:31:37Z

@cpina yeah, hoping to get to it this weekend :)

cpina · 2025-02-27T17:42:36Z

@cpina yeah, hoping to get to it this weekend :)

Super great news, thanks very much!

jamesoff added the build label Feb 4, 2024

This was referenced Feb 18, 2024

Create dir for html test output #1350

Merged

Attempt to fix TZ-related test failures #1351

Merged

wyardley mentioned this issue Feb 8, 2025

test(alerter): fix time and timezone issues in alerter tests #1598

Merged

jamesoff closed this as completed in #1598 Feb 15, 2025

jamesoff closed this as completed in 7dd6352 Feb 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit test execution #1330

Unit test execution #1330

cpina commented Jan 23, 2024 •

edited

Loading

jamesoff commented Feb 4, 2024

cpina commented Feb 13, 2024

cpina commented Feb 14, 2024 •

edited

Loading

jamesoff commented Feb 14, 2024

cpina commented Feb 14, 2024

cpina commented Aug 6, 2024

jamesoff commented Aug 11, 2024

wyardley commented Feb 8, 2025

cpina commented Feb 27, 2025

jamesoff commented Feb 27, 2025

cpina commented Feb 27, 2025

Unit test execution #1330

Unit test execution #1330

Comments

cpina commented Jan 23, 2024 • edited Loading

jamesoff commented Feb 4, 2024

cpina commented Feb 13, 2024

cpina commented Feb 14, 2024 • edited Loading

jamesoff commented Feb 14, 2024

cpina commented Feb 14, 2024

cpina commented Aug 6, 2024

jamesoff commented Aug 11, 2024

wyardley commented Feb 8, 2025

cpina commented Feb 27, 2025

jamesoff commented Feb 27, 2025

cpina commented Feb 27, 2025

cpina commented Jan 23, 2024 •

edited

Loading

cpina commented Feb 14, 2024 •

edited

Loading