-
Notifications
You must be signed in to change notification settings - Fork 342
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
Skip display rule option #590
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,6 +190,50 @@ TEST test_queue_notification_close_histignore(void) | |
PASS(); | ||
} | ||
|
||
TEST test_queue_notification_skip_display(void) | ||
{ | ||
struct notification *n; | ||
|
||
// Test skipping display | ||
n = test_notification("n", -1); | ||
n->skip_display = true; | ||
|
||
queues_init(); | ||
queues_notification_insert(n); | ||
QUEUE_LEN_ALL(1, 0, 0); | ||
queues_update(STATUS_NORMAL); | ||
QUEUE_LEN_ALL(0, 0, 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly, I'm missing another check for the signal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we check for the signal? Note that this code is exercising the queues, not end-to-end dbus functionality (the signal is done in |
||
queues_teardown(); | ||
|
||
PASS(); | ||
} | ||
|
||
TEST test_queue_notification_skip_display_redisplayed(void) | ||
{ | ||
struct notification *n; | ||
|
||
// Test skipping display | ||
n = test_notification("n", -1); | ||
n->skip_display = true; | ||
|
||
queues_init(); | ||
queues_notification_insert(n); | ||
QUEUE_LEN_ALL(1, 0, 0); | ||
queues_update(STATUS_NORMAL); | ||
QUEUE_LEN_ALL(0, 0, 1); | ||
|
||
queues_history_pop(); | ||
QUEUE_LEN_ALL(1, 0, 0); | ||
queues_update(STATUS_NORMAL); | ||
QUEUE_CONTAINSm("A skip display notification should stay in displayed " | ||
"queue when it got pulled out of history queue", | ||
DISP, n); | ||
|
||
queues_teardown(); | ||
|
||
PASS(); | ||
} | ||
|
||
TEST test_queue_history_overfull(void) | ||
{ | ||
settings.history_length = 10; | ||
|
@@ -714,6 +758,8 @@ SUITE(suite_queues) | |
RUN_TEST(test_queue_length); | ||
RUN_TEST(test_queue_notification_close); | ||
RUN_TEST(test_queue_notification_close_histignore); | ||
RUN_TEST(test_queue_notification_skip_display); | ||
RUN_TEST(test_queue_notification_skip_display_redisplayed); | ||
RUN_TEST(test_queue_stacking); | ||
RUN_TEST(test_queue_stacktag); | ||
RUN_TEST(test_queue_teardown); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hunk seems redundant, the notifications never see the displayed queue but skip directly from waiting to history. Was there a reason for adding this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, would need to ask @bebehei as he wrote this. It does seem redundant for the current use of
queues_notification_is_finished
, but if there are future usages of it, it might be good to have this check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may seem redundant, but actually it isn't. For the case, that an already displayed notification gets replaced, this would trigger the immediate removal.