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

tests: fix describe topics w/ docs and types #7877

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

NyaliaLui
Copy link
Contributor

@NyaliaLui NyaliaLui commented Dec 20, 2022

Cover letter

This test was failing because of poor STDOUT parsing that assumed a fixed number of lines. This commit removes those assumptions and uses regular expressions to parse STDOUT instead of string.split()

Fixes #7851

Changes from force-push 81ce31c:

  • Yank out the line for redpanda.datapolicy from the property table and check it on its own
  • Parse output until we reach the end of the property table. Then parse the documentation section
  • name in properties.keys() can be rewritten as name in properties
  • Check the documentation section using indexes instead of an iterator

Changes from force-push 24792e4:

  • For redpanda.datapolicy, check for default_config is sufficient
  • Use for line in output instead of an iterator so the StopIterator is automatically handled

Changes from force-push b80b818 and b11b7d8:

  • Use enumerate to find the index of the table separator line
  • Assert that the docs section is there

Backports Required

  • none - not a bug fix
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v22.3.x
  • v22.2.x
  • v22.1.x

UX Changes

  • none

Release Notes

  • none

tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
Copy link
Contributor

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge improvements, left two small comments, nice work!

tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Outdated Show resolved Hide resolved
tests/rptest/tests/describe_topics_test.py Show resolved Hide resolved
graphcareful
graphcareful previously approved these changes Dec 22, 2022
Copy link
Contributor

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tests/rptest/tests/describe_topics_test.py Show resolved Hide resolved
This test was failing because of poor STDOUT parsing that assumed a
fixed number of lines. This commit removes those assumptions and uses
regular expressions to parse STDOUT instead of string.split()

Fixes redpanda-data#7851
@NyaliaLui
Copy link
Contributor Author

CI failure is #7874 which should be fixed upstream now

I'm restarting the K8s-operator and golinter

Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@NyaliaLui for the future: we control the wrappers around clients (e.g. KCL) so we can add the sort of parsing you are doing in this test to the client wrapper itself so that other users of the client don't accidentally recreate all the parsing later on.

@dotnwat dotnwat merged commit 3be949b into redpanda-data:dev Dec 23, 2022
@NyaliaLui NyaliaLui deleted the describe-topics-fix branch March 2, 2023 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI failure in DescribeTopicsTest.test_describe_topics_with_documentation_and_types
4 participants