search

dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.42k stars 338 forks source link

Assert that notification icon is not null in tests #1269

Closed bynect closed 5 months ago

bynect commented 5 months ago

This is a temporary fix to #1228 until we figure out what causes the issue in first place (icons not being loaded).

This simply adds assertion so that if the icon is null the test suite will not segfault

The ci will fail...

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2fcea84) 65.45% compared to head (417dae3) 65.47%.

:exclamation: Current head 417dae3 differs from pull request most recent head a3bfee8. Consider uploading reports for the commit a3bfee8 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1269 +/- ## ========================================== + Coverage 65.45% 65.47% +0.02% ========================================== Files 46 46 Lines 7749 7754 +5 ========================================== + Hits 5072 5077 +5 Misses 2677 2677 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1269/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1269/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `65.47% <100.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bynect commented 5 months ago

The ci actually failed with another error unrelated to the changes and is not reproducible locally... quite the mystery

fwsmit commented 5 months ago

I think the memory leak was addressed by @zappolowski in #1274

fwsmit commented 5 months ago

This change seems fine to me. Although the tests will probably not pass locally for you now then?

bynect commented 5 months ago

This change seems fine to me. Although the tests will probably not pass locally for you now then?

Yes they fail but I still don't know why... but at least no segfault

apprehensions commented 4 months ago

It is a better idea to skip the test if the icon is missing.

bynect commented 4 months ago

It is a better idea to skip the test if the icon is missing.

do you know how to do that? I am not an expert of greatest

fwsmit commented 4 months ago

It should be SKIP()