Convert remaining KafkaConsumer tests to pytest
#1886
Conversation
|
This is in preparation for #1193. |
jeffwidman
force-pushed
the
convert-remaining-unittests-to-pytest
branch
7 times, most recently
from
25b9ed6
to
ff36ea0
Compare
| seen_partitions.add(partition) | ||
|
|
||
| # Check that we fetched at least 1 message from both partitions | ||
| assert seen_partitions == {TopicPartition(topic, 0), TopicPartition(topic, 1)} |
There was a problem hiding this comment.
I'm seeing occasional test failures here.
I actually don't see the point of this test--when @tvoinarovskyi originally wrote it, it also tested that we were getting 3K bytes back, but that is no longer true in Kafka 0.11, so @dpkp removed that part of the test.
It appears to be verifying no partition starvation, and sometimes it works and sometimes it doesn't, but 10 poll cycles feels a little short to guarantee that we receive from both partitions... maybe increase the cycle count to 100 and not worry about the very infrequent failure?
Personally, I'd rather merge with test_kafka_consumer_max_bytes_one_msg() and verify that even if we are sending larger messages to two partitions that we can consume from both partitions and don't get starvation...
Perhaps this change should be saved for a separate PR and in this one just get the tests to pass by re-running the occasional failure and then merge as-is.
There was a problem hiding this comment.
Yea, I don't remember the context under which I did this test. Feels like it doesn't test much to begin with. I think I wanted to verify that large messages will pass through even with a low max_bytes.
Feel free to merge it as you see fit. Sorry for the trouble again with this test.
There was a problem hiding this comment.
Looking at your original code, the intent was good, just Kafka 0.11 changed behavior a bit so this test got munged around. I'll try to merge it with the following test in a followup PR.
jeffwidman
force-pushed
the
convert-remaining-unittests-to-pytest
branch
from
ff36ea0
to
b8d9296
Compare
This makes it so the only remaining use of `unittest` is in the old tests of the deprecated `Simple*` clients. All `KafkaConsumer` tests are migrated to `pytest`.
Locally I can Kafka 1.0.1 with python 3.7 to pass intermittently. On Travis it's not passing. So wondering if I just need more iterations. Hopefully we only need 20-50, but trying 90 here to see if that truly fixes it without actually going over the 100 messages we sent... ie, cn we fetch from both partitions or does it force us to do 100 messages from first partition before going onto second partition (which would be a problem)?? Note: not guaranteed that 90 cycles isn't getting us all the 100 messages in the first partition since `fetch_max_bytes=300`... but a shot in the dark here... if Travis still fails even with this, then may be something else is the problem.
jeffwidman
force-pushed
the
convert-remaining-unittests-to-pytest
branch
from
b8d9296
to
f370d97
Compare
This makes it so the only remaining use of
unittestis in the oldtests of the deprecated
Simple*clients. AllKafkaConsumertests aremigrated to
pytest.This change is