anton-liauchuk commented on PR #16143:
URL: https://github.com/apache/kafka/pull/16143#issuecomment-2158416663

   > Hi @anton-liauchuk Thanks for the PR!
   > 
   > Is the KAFKA ticket linked the right one? I don't think this 
implementation matches the description of that ticket. I expected changes to 
the FakeLocalMetadataStore to be necessary.
   
   From the task description:
   
   > These tests and/or the metadata store should be changed so that the tests 
are isolated from one another, and actually perform the assertions that 
correspond to their titles.
   
   I made changes to the tests to ensure they assert a specific operation. 
Before my changes, the test
   
`org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest#testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin`
 might have passed due to refresh.topics.enabled flag and related 
configuration. After the new changes, this test asserts the behavior of 
`org.apache.kafka.connect.mirror.MirrorSourceConnector#computeAndCreateTopicPartitions`
 without depending on other functionality.
   
   The test 
`org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest#testSyncTopicConfigUseProvidedForwardingAdmin`
 did not properly check the sync functionality because there were no configs 
for the topic and no related assertions. With the recent changes, I updated the 
test to check the `retention.bytes` config to ensure that it was properly 
synced. To accomplish this, I had to implement 
`org.apache.kafka.connect.mirror.clients.admin.FakeForwardingAdminWithLocalMetadata#incrementalAlterConfigs`
 to store new configs during the sync.
   
   For now, it seems that these two tests were the main issue in this test 
class. With the new changes, each test will assert only one particular 
functionality.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to