-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add field to change HTTP port #13479
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
Changes from 1 commit
c7798cd
1ce282e
157c573
df53f2f
59c2e07
9f04c77
47aeffd
92a0887
987ed00
bb33695
a569a88
6ca44e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,20 +13,31 @@ class RemotePreferencesTest { | |
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| preferences = new RemotePreferences(1000, true, false); | ||
| preferences = new RemotePreferences(1000, true, 3000, false); | ||
| } | ||
|
|
||
| @Test | ||
| void getPort() { | ||
| assertEquals(1000, preferences.getPort()); | ||
| } | ||
|
|
||
| @Test | ||
| void getHttpPort() { | ||
| assertEquals(3000, preferences.getHttpPort()); | ||
| } | ||
|
|
||
| @Test | ||
| void setPort() { | ||
| preferences.setPort(2000); | ||
| assertEquals(2000, preferences.getPort()); | ||
| } | ||
|
|
||
| @Test | ||
| void setHttpPort() { | ||
| preferences.setHttpPort(4000); | ||
| assertEquals(4000, preferences.getHttpPort()); | ||
| } | ||
|
|
||
| @Test | ||
| void useRemoteServer() { | ||
| assertTrue(preferences.useRemoteServer()); | ||
|
|
@@ -43,8 +54,18 @@ void isDifferentPortTrue() { | |
| assertTrue(preferences.isDifferentPort(2000)); | ||
| } | ||
|
|
||
| @Test | ||
| void isDifferentHttpPortTrue() { | ||
| assertTrue(preferences.isDifferentHttpPort(4000)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using assertTrue with a boolean condition instead of asserting the actual values. Should use assertEquals to compare the actual HTTP port values for better test clarity and error messages. |
||
| } | ||
|
|
||
| @Test | ||
| void isDifferentPortFalse() { | ||
| assertFalse(preferences.isDifferentPort(1000)); | ||
| } | ||
|
|
||
| @Test | ||
| void isDifferentHttpPortFalse() { | ||
| assertFalse(preferences.isDifferentHttpPort(3000)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using assertFalse with a boolean condition instead of asserting the actual values. Should use assertEquals to compare the actual HTTP port values for better test clarity and error messages. |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also add
on portto the end of the text to clarify that the integer value refers to a portThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about line 88? Would the id name
httpServerPortbe enough to clarify that's it's a port?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that information is not visible to the user, i mean the text that is being displayed to the user. So it would be better if it looked more like the one above for the single instance

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! Okay, I understand now.