-
Notifications
You must be signed in to change notification settings - Fork 11
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
Flags: check for corrrect untagged flags responses from STORE/FETCH #142
Flags: check for corrrect untagged flags responses from STORE/FETCH #142
Conversation
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.
Looks good. We will have to revisit anyway when we have 'admin == owner', so no problem with the XXXed out tests.
# XXX when setting them as admin, suggesting they were already set, | ||
# XXX and there's no explicit test that they were missing before being | ||
# XXX set... | ||
|
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.
I wonder if this is due to sharedseen vs not sharedseen?
Our goal eventually is to have admin act as "owner" anyway.
$msg{A}->set_attribute(flags => []); | ||
$self->check_messages(\%msg); | ||
} | ||
|
||
xlog $self, "Cannot set one more wafer-thin user flag"; | ||
my $flag = '$Farnarkle'; | ||
$self->assert_null($allflags{$flag}); | ||
my $r = $talk->store('1', '+flags', "($flag)"); |
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.
Yeah, good idea - $r
is a bad name.
I think I also want these to check modseq before/after setting \Seen to make sure it's only bumped once, because the PR that "fixes" the issue may double bump it, and that wouldn't be good. |
bb4e647
to
69bb304
Compare
Moved to cyrusimap/cyrus-imapd#3829 |
Two commits here: one is a new regression test for cyrusimap/cyrus-imapd#3240 / cyrusimap/cyrus-imapd#3577, the other adds similar hardening to the tests that already existed.
I have a bunch of XXX comments in here on some weirdnesses that turned up that weren't directly relevant to the rabbit I was chasing. If you have thoughts on those I'd love to hear them, but otherwise I'll probably kick them down the road for now so I can get back to other things.