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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

(test) Add test for InstanceSpec.<init> and .getQuorumPort #418

Closed
wants to merge 1 commit into from

Conversation

lacinoire
Copy link

Hey 馃槉
I want to contribute the following test:

Test that spec.getQuorumPort() is equal to 2.
This tests the methods InstanceSpec.<init> and InstanceSpec.getQuorumPort.
This test is based on the test setCustomTickTimeTest.

Curious to hear what you think!

(I wrote this test as part of a research study at TU Delft. Find out more)

@tisonkun
Copy link
Member

This patch seems like an explicit test of getter/setter. Basically it's meaningless and we don't test such methods.

Could you elaborate why you'd like to add such a test?

@lacinoire
Copy link
Author

Good point regarding the getter method.
I still proposed the test case because it also covers new instructions in the constructor of InstanceSpec (setting the quorum port to the passed value if it is larger than 0).

Do you find it meaningful to test (this kind of) constructor?

@tisonkun
Copy link
Member

Closed as we don't test for simple getter/setter.

If you're interested in setting port correctly, you may take a look at CURATOR-535, related patches:

@tisonkun tisonkun closed this Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants