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

rptest/simple_connect_test: refactor out mount/unmount tests #25036

Merged

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Feb 5, 2025

Refactors simple_connect_test, pulling out the mount/unmount tests into
a separate file, and updating both test files to use new reusable
primitives for setting up RPCN and schemas.

I also tweaked the structure to move setup to init/setUp methods and to
avoid the test-body-as-lambdas pattern, since I found them a bit
unnatural to read through.

There are no major functional changes to the tests. I wanted to separate
these tests because I intend on adding additional tests for
mount/unmount functionality.

Backports Required

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

Release Notes

  • none

Adds a method to make an RPCN counter stream config. Such a stream was
already used in a couple places, and the configuration is quite a
mouthful to write.
It's nice for test authors to not have to think about writing schema
files, so this encapsulates some logic I found in simple_connect_test to
create a schema using a tempfile.
Refactors simple_connect_test, pulling out the mount/unmount tests into
a separate file, and updating both test files to use new reusable
primitives for setting up RPCN and schemas.

I also tweaked the structure to move setup to init/setUp methods and to
avoid the test-body-as-lambdas pattern, since I found them a bit
unnatural to read through.

There are no major functional changes to the tests. I wanted to separate
these tests because I intend on adding additional tests for
mount/unmount functionality.
self.wait_for_migration_states(out_migration_id, ['finished'])
self.wait_partitions_disappear([self.TOPIC_NAME])

verifier.wait(progress_timeout_sec=10 * self.SLOW_COMMIT_INTVL_S)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bashtanov on my local machine, this test ran for 12 minutes. I don't believe this is a result of my changes here -- is this call to verifier.wait(10min) actually what you want? It appears to be waiting that long, and it doesn't throw on timeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

ping @bashtanov

@vbotbuildovich
Copy link
Collaborator

CI test results

test results on build#61595
test_id test_kind job_url test_status passed
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery ducktape https://buildkite.com/redpanda/redpanda/builds/61595#0194d45b-7355-4170-aa5e-f0c20004a185 FLAKY 1/4
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery ducktape https://buildkite.com/redpanda/redpanda/builds/61595#0194d45e-dace-4f22-9ee6-d691d1c87fd9 FLAKY 1/2
rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance ducktape https://buildkite.com/redpanda/redpanda/builds/61595#0194d45e-dacd-4b9d-9819-c08ed8f2fef1 FLAKY 1/3
rptest.tests.scaling_up_test.ScalingUpTest.test_scaling_up_with_recovered_topic ducktape https://buildkite.com/redpanda/redpanda/builds/61595#0194d45e-dacc-4203-adc5-bd9b6c197634 FLAKY 1/3

@andrwng andrwng enabled auto-merge February 5, 2025 09:34
Copy link
Contributor

@bashtanov bashtanov left a comment

Choose a reason for hiding this comment

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

LGTM. There seems to be a bug in my test that leads to this timeout, I'll have a look

@andrwng andrwng merged commit 63d914e into redpanda-data:dev Feb 5, 2025
17 checks passed
@andrwng
Copy link
Contributor Author

andrwng commented Feb 5, 2025

LGTM. There seems to be a bug in my test that leads to this timeout, I'll have a look

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants