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

Add back necessary tests for deprecated settings that are replaced during applying inclusive naming #2805

Closed
tlfeng opened this issue Apr 7, 2022 · 2 comments · Fixed by #2825
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request :test Adding or fixing a test v2.1.0 Issues and PRs related to version 2.1.0

Comments

@tlfeng
Copy link
Collaborator

tlfeng commented Apr 7, 2022

Is your feature request related to a problem? Please describe.
Issue #2769 shows a fault caused by PR #2463 (Deprecate setting 'cluster.initial_master_nodes' and introduce the alternative setting 'cluster.initial_cluster_manager_nodes' ).
The backwards compatibility was broken by mistake.
In the above PR, the deprecated setting name is replaced directly by the new setting name in the existing tests, while new unit tests for validating backwards compatibility didn't cover all the cases.
The defect should be avoided.

Describe the solution you'd like

Describe alternatives you've considered
none.

Additional context
none.

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request untriaged v2.0.0 Version 2.0.0 :test Adding or fixing a test v3.0.0 Issues and PRs related to version 3.0.0 and removed untriaged labels Apr 7, 2022
@anasalkouz
Copy link
Member

Are we going to duplicate the test to cover old and new usages? Can you create another issue to clean up those duplicate tests on 3.0?

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 7, 2022

@anasalkouz Yes. I will revise all the changes in existing PRs, and duplicate the test to cover old and new usages.
While I'm not simply adding back every test for the old setting, but refine and pick the necessary tests that cover the missing part, by understanding the meaning of the removed test cases.
I think the tests can be removed along with removing the deprecated settings, and I will create an issue for the removal together.
We don't need to worry much about forgetting moving the tests in the future, because they include references to the deprecated setting, and the tests will be failed to compile after the deprecated usage is removed. Even if there is no reference, I will add TODO: in the code. 😄

@anasalkouz anasalkouz added v2.1.0 Issues and PRs related to version 2.1.0 and removed v2.0.0 Version 2.0.0 v3.0.0 Issues and PRs related to version 3.0.0 labels Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request :test Adding or fixing a test v2.1.0 Issues and PRs related to version 2.1.0
Projects
None yet
2 participants