-
Notifications
You must be signed in to change notification settings - Fork 594
fix(server): ensure repo disconnection on server start exit
#3980
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
Conversation
Removes '--password=xxx' parameter, which causes a server start failure due to incorrect repo password, and not for the case being checked, which is the lack of the "--insecure" parameter.
TestServerStartInsecureTestServerStartInsecure
a9185b4 to
d1b7a92
Compare
TestServerStartInsecureserver start
server startserver start exit
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3980 +/- ##
==========================================
+ Coverage 75.86% 77.25% +1.39%
==========================================
Files 470 481 +11
Lines 37301 28806 -8495
==========================================
- Hits 28299 22255 -6044
+ Misses 7071 4655 -2416
+ Partials 1931 1896 -35 ☔ View full report in Codecov by Sentry. |
This was caught as a result of fixing a test.
d1b7a92 to
06229ed
Compare
| e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--without-password", "--tls-generate-cert") | ||
|
|
||
| // server fails to start when TLS is not configured and `--insecure` is not specified | ||
| e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0") |
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.
This is the modified test case, see first commit in this PR. It was:
e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--password=foo")| // server fails to start without a password but with TLS. | ||
| e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--tls-generate-cert", "--without-password") | ||
| // server fails to start with --without-password when `--insecure` is not specified | ||
| e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--without-password") // without TLS |
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.
Code movement only to group these cases. This was originally the last case.
|
@redgoat650 @plar PTAL |
redgoat650
left a comment
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.
LGTM
|
@jkowalski FYI |
Ensure repository disconnection at the end of the
server startCLI command.This was caught as a result of fixing the test below.
Fix
TestServerStartInsecure:Remove
--password=xxxparameter, which causes a server start failure due to incorrect repo password, and not for the case being checked, which is the lack of the--insecureparameter.Update test comments accordingly.