Skip to content
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

Intermittent test failure in one of Bodhi's fedora-messaging tests #2858

Closed
bowlofeggs opened this issue Dec 17, 2018 · 1 comment · Fixed by #2860
Closed

Intermittent test failure in one of Bodhi's fedora-messaging tests #2858

bowlofeggs opened this issue Dec 17, 2018 · 1 comment · Fixed by #2860
Labels
Crash Issues related to an unhandled crash Critical We can't go on living in this sqalor, drop everything and fix it! Tests Issues pertaining to Bodhi's tests

Comments

@bowlofeggs
Copy link
Contributor

There's a test assertion in Bodhi that occasionally fails like this:

bodhi/tests/server/test_notifications.py .......F

=================================== FAILURES ===================================
_____________________ TestPublish.test_fedmsg_publish_off ______________________

self = <bodhi.tests.server.test_notifications.TestPublish testMethod=test_fedmsg_publish_off>
mock_init = <MagicMock name='init' id='140416901561296'>

    @mock.patch.dict('bodhi.server.config.config', {'fedmsg_enabled': False})
    def test_fedmsg_publish_off(self, mock_init):
        """Assert when fedmsg publish is off, fedora-messaging messages are queued."""
        expected_msgs = [api.Message(topic='org.fedoraproject.dev.bodhi.demo.topic',
                                     body={u'such': 'important'})]
        session = Session()
    
        notifications.publish('demo.topic', {'such': 'important'})
    
        self.assertIn('messages', session.info)
        for expected, actual in zip(expected_msgs, session.info['messages']):
>           self.assertEqual(expected, actual)
E           AssertionError: Message(id='7e76630d-3e13-4478-9e82-f4f4a5975c0e', topic='org.fedoraproject.dev.bodhi.demo.topic', body={u'such': 'important'}) != Message(id='74855ca5-6638-472b-9018-6e5bde011267', topic='org.fedoraproject.dev.bodhi.demo.topic', body={u'such': u'important'})

bodhi/tests/server/test_notifications.py:141: AssertionError

The problem seems to be that the Message.id attribute is not the same between the two messages. We could fix it by either stopping to compare the id, or maybe by contributing an assertion method to fedora-messaging that allows users to compare messages without having to compare the ids.

@jeremycline what would you recommend here?

@bowlofeggs bowlofeggs added Critical We can't go on living in this sqalor, drop everything and fix it! Tests Issues pertaining to Bodhi's tests Crash Issues related to an unhandled crash labels Dec 17, 2018
@jeremycline
Copy link
Member

I think the problem is that the automatically-set sent-at header is included in the equality check which would explain the intermittent failure. The id shouldn't be included in the equality check and is just a uuid4 so it's quite likely it's never equal.

I can make a PR to delete the header for that bodhi test so you're unblocked and also fix the equality check in fedora-messaging.

jeremycline added a commit to jeremycline/bodhi that referenced this issue Dec 17, 2018
This header is the time when the message was sent. If the test runs
quickly enough the two time stamps could be equal, which is why this
doesn't always fail.

fixes fedora-infra#2858

Signed-off-by: Jeremy Cline <jcline@redhat.com>
jeremycline added a commit to jeremycline/fedora-messaging that referenced this issue Dec 17, 2018
In fedora-infra/bodhi#2858 it was noted that
tests involving equality of messages fails because of the sent-at
header. It doesn't make a lot of sense to include this in an equality
check for the same reason the id is not included in the check - it's
metadata and of little interest to developers (unless they are tracking
messages).

Signed-off-by: Jeremy Cline <jcline@redhat.com>
jeremycline added a commit to jeremycline/fedora-messaging that referenced this issue Dec 17, 2018
In fedora-infra/bodhi#2858 it was noted that
tests involving equality of messages fails because of the sent-at
header. It doesn't make a lot of sense to include this in an equality
check for the same reason the id is not included in the check - it's
metadata and of little interest to developers (unless they are tracking
messages).

Signed-off-by: Jeremy Cline <jcline@redhat.com>
jeremycline added a commit to jeremycline/bodhi that referenced this issue Dec 19, 2018
This header is the time when the message was sent. If the test runs
quickly enough the two time stamps could be equal, which is why this
doesn't always fail.

fixes fedora-infra#2858

Signed-off-by: Jeremy Cline <jcline@redhat.com>
jeremycline added a commit to jeremycline/fedora-messaging that referenced this issue Dec 19, 2018
In fedora-infra/bodhi#2858 it was noted that
tests involving equality of messages fails because of the sent-at
header. It doesn't make a lot of sense to include this in an equality
check for the same reason the id is not included in the check - it's
metadata and of little interest to developers (unless they are tracking
messages).

Signed-off-by: Jeremy Cline <jcline@redhat.com>
mergify bot pushed a commit to fedora-infra/fedora-messaging that referenced this issue Dec 24, 2018
In fedora-infra/bodhi#2858 it was noted that
tests involving equality of messages fails because of the sent-at
header. It doesn't make a lot of sense to include this in an equality
check for the same reason the id is not included in the check - it's
metadata and of little interest to developers (unless they are tracking
messages).

Signed-off-by: Jeremy Cline <jcline@redhat.com>
bowlofeggs pushed a commit to jeremycline/bodhi that referenced this issue Jan 3, 2019
This header is the time when the message was sent. If the test runs
quickly enough the two time stamps could be equal, which is why this
doesn't always fail.

fixes fedora-infra#2858

Signed-off-by: Jeremy Cline <jcline@redhat.com>
bowlofeggs pushed a commit to jeremycline/bodhi that referenced this issue Jan 4, 2019
This header is the time when the message was sent. If the test runs
quickly enough the two time stamps could be equal, which is why this
doesn't always fail.

fixes fedora-infra#2858

Signed-off-by: Jeremy Cline <jcline@redhat.com>
@mergify mergify bot closed this as completed in #2860 Jan 5, 2019
mergify bot pushed a commit that referenced this issue Jan 5, 2019
This header is the time when the message was sent. If the test runs
quickly enough the two time stamps could be equal, which is why this
doesn't always fail.

fixes #2858

Signed-off-by: Jeremy Cline <jcline@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crash Issues related to an unhandled crash Critical We can't go on living in this sqalor, drop everything and fix it! Tests Issues pertaining to Bodhi's tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants