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
udprunner + build improvements + timeout changes #452
Conversation
} | ||
|
||
// Close closes the last connection and returns the total number of sockets used for the run. | ||
func (c *UDPClient) Close() int { |
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.
Similar blocks of code found in 3 locations. Consider refactoring.
return &c, nil | ||
} | ||
|
||
func (c *UDPClient) connect() (net.Conn, error) { |
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.
Similar blocks of code found in 2 locations. Consider refactoring.
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
Method UDPClient.Fetch
has 8 return statements (exceeds 4 allowed).
|
||
// RunUDPTest runs an tcp test and returns the aggregated stats. | ||
// Some refactoring to avoid copy-pasta between the now 3 runners would be good. | ||
func RunUDPTest(o *RunnerOptions) (*RunnerResults, error) { |
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.
Similar blocks of code found in 2 locations. Consider refactoring.
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
Similar blocks of code found in 2 locations. Consider refactoring.
Code Climate has analyzed commit bc5d1ea and detected 13 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
} | ||
|
||
// NewUDPClient creates and initialize and returns a client based on the UDPOptions. | ||
func NewUDPClient(o *UDPOptions) (*UDPClient, error) { |
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.
Similar blocks of code found in 2 locations. Consider refactoring.
|
||
// Run tests tcp request fetching. Main call being run at the target QPS. | ||
// To be set as the Function in RunnerOptions. | ||
func (udpstate *RunnerResults) Run(t int) { |
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.
Similar blocks of code found in 2 locations. Consider refactoring.
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
Method UDPClient.Fetch
has 53 lines of code (exceeds 50 allowed). Consider refactoring.
|
||
// RunUDPTest runs an tcp test and returns the aggregated stats. | ||
// Some refactoring to avoid copy-pasta between the now 3 runners would be good. | ||
func RunUDPTest(o *RunnerOptions) (*RunnerResults, error) { |
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.
Function RunUDPTest
has 52 lines of code (exceeds 50 allowed). Consider refactoring.
|
||
// TODO: this quite the search and replace tcp->udp from tcprunner/ - refactor? | ||
|
||
type UDPResultMap map[string]int64 |
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.
exported type UDPResultMap should have comment or be unexported
Codecov Report
@@ Coverage Diff @@
## master #452 +/- ##
========================================
- Coverage 86.7% 86.2% -0.6%
========================================
Files 25 26 +1
Lines 2860 2982 +122
========================================
+ Hits 2480 2569 +89
- Misses 252 271 +19
- Partials 128 142 +14
Continue to review full report at Codecov.
|
udprunner/udprunner.go
Outdated
errMismatch = fmt.Errorf("read not echoing writes") | ||
) | ||
|
||
// Generates a 24 bytes unique payload for each runner thread and message sent. |
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.
comment on exported function GeneratePayload should be of the form "GeneratePayload ..."
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
exported method UDPClient.Fetch should have comment or be unexported
…so use 750ms as new default timeout. for http commandline the new default is 3s (from 15s)
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
Method UDPClient.Fetch
has 9 return statements (exceeds 4 allowed).
return socket, nil | ||
} | ||
|
||
func (c *UDPClient) Fetch() ([]byte, error) { |
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.
Method UDPClient.Fetch
has 56 lines of code (exceeds 50 allowed). Consider refactoring.
|
||
// TODO: this quite the search and replace udp->udp from tcprunner/ - refactor? | ||
|
||
var UDPTimeOutDefaultValue = 750 * time.Millisecond |
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.
exported var UDPTimeOutDefaultValue should have comment or be unexported
Fixes #446