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

Fix config diff test #5530

Conversation

sknot-rh
Copy link
Member

@sknot-rh sknot-rh commented Sep 6, 2021

Type of change

  • Bugfix

Description

Use reflection to call package-protected ctor to fix tests.

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

Signed-off-by: Stanislav Knot <sknot@redhat.com>
@sknot-rh sknot-rh added this to the 0.26.0 milestone Sep 6, 2021
@sknot-rh sknot-rh requested a review from a team September 6, 2021 11:02
Comment on lines 275 to +277
ces.add(new ConfigEntry("inter.broker.listener.name", "david"));
ces.add(new ConfigEntry("group.min.session.timeout.ms", "42"));
ces.add(new ConfigEntry("host.name", "honza"));
ces.add(new ConfigEntry("zookeeper.sync.time.ms", "8000"));
Copy link
Member

Choose a reason for hiding this comment

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

Can we add comments for why we chose these three configs specifically?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no specific reason. We are doing just 3 random changes. Property host.name was removed from kafka 3.0.0 config model, so we just switched to another property.

Copy link
Member

Choose a reason for hiding this comment

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

Might be still worth a comment that they are randomly chosen without any deeper meaning ... even that is an information which might be useful.

Signed-off-by: Stanislav Knot <sknot@redhat.com>
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Do you need to uncomment the @Disabled to actually run these tests in the CI?

Signed-off-by: Stanislav Knot <sknot@redhat.com>
Signed-off-by: Stanislav Knot <sknot@redhat.com>
@scholzj scholzj merged commit 1f8a6a0 into strimzi:add-support-for-kafka-3.0.0 Sep 6, 2021
@sknot-rh sknot-rh deleted the kafka300-fix-config-diff-test branch September 7, 2021 05:14
scholzj pushed a commit to scholzj/strimzi-kafka-operator that referenced this pull request Sep 20, 2021
* Fix KafkaBrokerConfigurationDiffTest

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* reenable

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>
scholzj pushed a commit that referenced this pull request Sep 23, 2021
* Fix KafkaBrokerConfigurationDiffTest

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* reenable

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>
scholzj added a commit that referenced this pull request Sep 24, 2021
* Add support for Kafka 3.0.0

Signed-off-by: Jakub Scholz <www@scholzj.com>

* [Kafka 3.x] Remove deprecated methods from TO (#5522)

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* Rebase & Disable failing unit tests

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Fix one more test

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Fix mocking (#5527)

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* Fix TO ITs

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Fix config diff test (#5530)

* Fix KafkaBrokerConfigurationDiffTest

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* reenable

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* review comments

Signed-off-by: Stanislav Knot <sknot@redhat.com>

* Close the EmbeddedKafkaClusters

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Fix TO build

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Update to 3.0.0 RC2

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Add Kafka 2.8.1 as well

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Update to final version of 3.0.0

Signed-off-by: Jakub Scholz <www@scholzj.com>

* Fix TopicST and update upgrade STs JSON

Signed-off-by: Jakub Scholz <www@scholzj.com>

* check if supported kafka versions contains version (#5606)

Signed-off-by: Lukas Kral <lukywill16@gmail.com>

* Review comments JSt

Signed-off-by: Jakub Scholz <www@scholzj.com>

Co-authored-by: Stanislav Knot <sknot@redhat.com>
Co-authored-by: Lukáš Král <53821852+im-konge@users.noreply.github.com>
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.

None yet

3 participants