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

Refactored SentinelGrpcServerInterceptorTest #101

Conversation

refactormachine
Copy link
Contributor

Performed extract class operation for SentinelGrpcServerInterceptorTest. Extracted fields: server. Extracted Methods: prepareServer, stopServer.

@CLAassistant
Copy link

CLAassistant commented Aug 28, 2018

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Aug 28, 2018

Codecov Report

Merging #101 into master will decrease coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #101      +/-   ##
============================================
- Coverage     45.98%   45.84%   -0.14%     
+ Complexity      557      556       -1     
============================================
  Files           113      114       +1     
  Lines          3806     3813       +7     
  Branches        530      531       +1     
============================================
- Hits           1750     1748       -2     
- Misses         1846     1854       +8     
- Partials        210      211       +1
Impacted Files Coverage Δ Complexity Δ
...a/csp/sentinel/slots/statistic/base/LeapArray.java 71.79% <0%> (-5.13%) 14% <0%> (-1%)
...libaba/csp/sentinel/slots/block/flow/FlowRule.java 58.77% <0%> (-1.59%) 31% <0%> (ø)
...ibaba/csp/sentinel/node/metric/MetricSearcher.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
...libaba/csp/sentinel/node/metric/MetricsReader.java 0% <0%> (ø) 0% <0%> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8a72c4...1add639. Read the comment docs.

GrpcTestServer() {
}

void prepare(int port) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Using the name start may be more appropriate.


import java.io.IOException;

import static org.junit.Assert.*;
Copy link
Member

Choose a reason for hiding this comment

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

You can remove unused imports.

@@ -43,8 +39,8 @@

private final String resourceName = "com.alibaba.sentinel.examples.FooService/anotherHello";
private final int threshold = 4;
private final GrpcTestServer extractedSentinelGrpcServerInterceptorTest = new GrpcTestServer();
Copy link
Member

Choose a reason for hiding this comment

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

The name is too long, just grpcTestServer is okay.

…t' into feature/refactored-sentinel-grpc-server-interceptor-test

# Conflicts:
#	sentinel-adapter/sentinel-grpc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/grpc/GrpcTestServer.java
}
ServerBuilder<?> serverBuild = ServerBuilder.forPort(port)
.addService(new FooServiceImpl());
if(shouldintercept)
Copy link
Member

Choose a reason for hiding this comment

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

Please reformat your code with Alibaba P3C Plugin and obey the rules in Alibaba Java Coding Guidelines (naming, indent, format, etc.). For example:

if (shouldIntercept) {
    serverBuilder.intercept(new SentinelGrpcServerInterceptor());
}

@refactormachine
Copy link
Contributor Author

Hi @sczyh30 , reformatted the code.

@sczyh30 sczyh30 merged commit b61be26 into alibaba:master Aug 30, 2018
@sczyh30
Copy link
Member

sczyh30 commented Aug 30, 2018

Nice, thanks for your contribution!

@sczyh30 sczyh30 removed the to-review To review label Aug 30, 2018
Arlmls pushed a commit to Arlmls/Sentinel that referenced this pull request Jan 8, 2019
- Added a GrpcTestServer to abstract common server logic
- Refactor with new added `GrpcTestServer`
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.

5 participants