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

Upgrade dependencies #545

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Upgrade dependencies #545

merged 2 commits into from
Apr 3, 2024

Conversation

Danielius1922
Copy link
Member

@Danielius1922 Danielius1922 commented Apr 2, 2024

Summary by CodeRabbit

  • New Features

    • Introduced the use of errors.New for error creation across various components for improved error handling consistency.
    • Updated context handling in Session structs to use atomic.Pointer[context.Context] for enhanced performance and safety.
  • Refactor

    • Replaced fmt.Errorf with errors.New in multiple places to standardize error creation.
    • Switched from experimental maps package to a non-experimental one in router components.
    • Enhanced test assertions for clarity and reliability across several test suites.
  • Bug Fixes

    • Corrected error assertions in test cases to ensure immediate failure on critical errors, improving test reliability.
    • Updated assertions to use more accurate comparison methods (require.Equal, require.NoError, require.ErrorIs) for better test clarity.
  • Chores

    • Enabled and modified various linters in .golangci.yml for improved code quality and consistency.

Copy link

coderabbitai bot commented Apr 2, 2024

Walkthrough

The changes in this update focus on enhancing error handling by replacing fmt.Errorf with errors.New, updating test assertion methods for stricter requirements, and making structural modifications like changing atomic.Value to atomic.Pointer[context.Context]. There is also an emphasis on improving code consistency and readability, especially around error generation and test assertions.

Changes

Files Change Summary
.golangci.yml, message/codes/code_string.go, message/codes/codes.go, message/options_test.go, message/pool/message_test.go, message/status/status_test.go Enhanced error handling and test assertions; replaced fmt with errors for error creation.
dtls/client_test.go, dtls/server/server.go, dtls/server/session.go, tcp/client/conn.go, tcp/client/session.go, udp/client/conn.go, udp/client/conn_test.go, udp/coder/bench_test.go, udp/coder/coder_test.go, udp/server/discover.go, udp/server/server.go, udp/server/session.go Replaced fmt.Errorf with errors.New; updated assertions in tests; changed atomic.Value to atomic.Pointer[context.Context].
mux/router.go, mux/router_test.go, pkg/sync/map.go Updated import statements due to dependency updates.
tcp/coder/bench_test.go, tcp/coder/coder_test.go Modified variable declarations and logic related to encoding.

Possibly related issues

  • Dependency Dashboard hub#418: The dependency updates and error handling improvements might align with the objectives to update various modules and improve code quality.

"In the realm of codes and bytes, where logic and magic takes flights,
A rabbit hopped, with fixes bright, under the moon's soft lights.
🌙✨
Errors now clear, with errors.New, as old woes bid adieu,
Asserts strict, and dependencies anew, a cleaner code in view.
🐇💻
So here's to changes, big and small, in the endless code's hall,
A hop, a skip, a joyous call, for progress, one and all."

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ce5dcc5 and 923bce5.
Files ignored due to path filters (2)
  • go.mod is excluded by !**/*.mod
  • go.sum is excluded by !**/*.sum
Files selected for processing (40)
  • .golangci.yml (5 hunks)
  • dtls/client_test.go (1 hunks)
  • dtls/server/server.go (2 hunks)
  • dtls/server/session.go (2 hunks)
  • message/codes/code_string.go (2 hunks)
  • message/codes/codes.go (2 hunks)
  • message/codes/codes_test.go (1 hunks)
  • message/option.go (3 hunks)
  • message/options_test.go (2 hunks)
  • message/pool/message_test.go (2 hunks)
  • message/status/status_test.go (3 hunks)
  • mux/router.go (1 hunks)
  • mux/router_test.go (1 hunks)
  • net/blockwise/blockwise.go (9 hunks)
  • net/blockwise/blockwise_test.go (5 hunks)
  • net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
  • net/connUDP.go (6 hunks)
  • net/connUDP_internal_test.go (3 hunks)
  • net/conn_test.go (3 hunks)
  • net/dtlslistener.go (1 hunks)
  • net/observation/handler.go (4 hunks)
  • net/tlslistener_test.go (4 hunks)
  • pkg/cache/cache_test.go (2 hunks)
  • pkg/sync/map.go (1 hunks)
  • server.go (2 hunks)
  • tcp/client/conn.go (1 hunks)
  • tcp/client/session.go (2 hunks)
  • tcp/client_test.go (1 hunks)
  • tcp/coder/bench_test.go (1 hunks)
  • tcp/coder/coder_test.go (1 hunks)
  • tcp/server/server.go (2 hunks)
  • udp/client/conn.go (2 hunks)
  • udp/client/conn_test.go (2 hunks)
  • udp/client_test.go (10 hunks)
  • udp/coder/bench_test.go (1 hunks)
  • udp/coder/coder_test.go (1 hunks)
  • udp/server/discover.go (3 hunks)
  • udp/server/server.go (4 hunks)
  • udp/server/session.go (2 hunks)
  • udp/server_test.go (1 hunks)
Additional Context Used
GitHub Check Runs (1)
test1_20 failure (3)

pkg/sync/map.go: [failure] 4-4:
package maps is not in GOROOT (/opt/hostedtoolcache/go/1.20.14/x64/src/maps)

Additional comments not posted (88)
message/status/status_test.go (2)

27-27: The replacement of errors.Is with require.ErrorIs in test assertions is a good practice. It ensures that the test fails immediately if the condition is not met, making the tests more robust and easier to debug.


36-36: Using require.ErrorIs for error comparison is consistent with the improvements made in error handling across the project. This change enhances the clarity and reliability of the test cases.

tcp/coder/bench_test.go (1)

15-15: Explicitly declaring enc as an int improves code readability by making the type of enc immediately clear. This is a minor but positive change for maintainability.

udp/coder/bench_test.go (1)

15-15: Just like in the TCP coder benchmark test, explicitly declaring enc as an int here enhances code clarity. Consistency in such improvements across similar files is commendable.

net/conn_test.go (7)

43-43: Switching from assert.NoError to require.NoError ensures that the test halts immediately if an error occurs, which is a best practice for critical error checks in tests.


46-46: Consistently using require.NoError for error handling in test teardown logic is a good practice, ensuring that any errors during cleanup are immediately addressed.


61-61: Applying require.NoError within the test's goroutine for closing connections enhances the test's reliability by ensuring immediate failure on errors.


68-68: Using require.NoError for error handling after dialing enhances the robustness of the test setup phase.


72-72: Ensuring no error occurs when closing the connection with require.NoError is crucial for clean test teardown. Good use of require for immediate failure on error.


80-80: Switching to require.Error for expected errors aligns with the goal of making tests fail fast on critical conditions. This change improves the test's clarity and reliability.


83-83: Consistent use of require.NoError for asserting the absence of errors throughout the test cases is a good practice, ensuring that tests fail fast upon encountering an error.

message/codes/code_string.go (1)

57-57: Replacing fmt.Errorf with errors.New for generating errors where no formatting is needed is a good practice. It simplifies the error creation process and slightly improves performance.

message/codes/codes_test.go (1)

55-55: Replacing require.True with require.Greater for asserting the length of a string provides a more precise and descriptive assertion. This change enhances the clarity and robustness of the test.

dtls/server/session.go (2)

22-22: Changing the ctx field type in the Session struct from atomic.Value to atomic.Pointer[context.Context] is a significant improvement. It leverages the type safety provided by atomic.Pointer, making the code more robust and type-safe.


93-93: The updated Context() method to return *s.ctx.Load() directly, thanks to the type safety of atomic.Pointer[context.Context], simplifies the code and avoids the need for type assertion. This is a cleaner and safer approach.

udp/server/discover.go (5)

5-5: Importing the errors package is appropriate for using errors.New for error creation.


13-13: Renaming the errors package to pkgErrors to avoid conflict with the standard library's errors package is a good practice when both are needed.


47-47: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.


51-51: Replacing fmt.Errorf with errors.New when no formatting is needed is a good improvement for error creation.


67-67: Correctly using a package-specific error (pkgErrors.ErrKeyAlreadyExists) ensures that errors are more descriptive and can be handled specifically.

udp/server/session.go (2)

21-21: Changing the ctx field type from atomic.Value to atomic.Pointer[context.Context] is a more type-safe approach for storing pointers to context.Context in an atomic way.


104-104: Using *s.ctx.Load() directly without type assertion is now possible due to the specific type atomic.Pointer[context.Context], enhancing code readability and safety.

message/codes/codes.go (1)

120-120: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

pkg/cache/cache_test.go (2)

121-121: Using require.Equal directly to compare the expected number of found elements with the actual count is a clear and concise way to assert test conditions.


148-148: Asserting that no elements are found after a LoadAndDeleteAll operation using require.Equal to check the count is a straightforward way to validate the cache's behavior.

net/dtlslistener.go (1)

73-73: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

server.go (2)

6-6: Importing the errors package is appropriate for using errors.New for error creation.


103-103: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.

mux/router_test.go (1)

4-4: Replacing the experimental golang.org/x/exp/maps package with the stable maps package is a good practice for ensuring long-term support and stability.

tcp/server/server.go (2)

85-85: Using errors.New for creating simple error messages without formatting is a good practice for consistency and performance.


138-138: Replacing fmt.Errorf with errors.New for a simple error message without formatting is a good practice for consistency and performance.

dtls/server/server.go (2)

92-92: The change from fmt.Errorf to errors.New for generating the error message in checkAndSetListener method aligns with the PR's objective to standardize error handling.


130-130: The change from fmt.Errorf to errors.New for generating the error message in the Serve method is consistent with the PR's goal to standardize error handling.

net/observation/handler.go (2)

5-5: The addition of the errors import aligns with the PR's objective to standardize error handling.


14-14: Renaming the errors package import to pkgErrors to avoid naming conflicts with the standard errors package is a sensible change.

tcp/client/session.go (1)

33-33: The update of the ctx field in the Session struct to use atomic.Pointer[context.Context] improves type safety and code clarity, especially in the Context() method where it simplifies context handling by removing the need for type assertion.

Also applies to: 141-141

net/client/limitParallelRequests/limitParallelRequests_test.go (2)

30-30: The change from fmt.Errorf to errors.New in the do method of the mockClient struct aligns with the PR's objective to standardize error handling.


35-35: The change from fmt.Errorf to errors.New in the doObserve method of the mockClient struct is consistent with the PR's goal to standardize error handling.

udp/coder/coder_test.go (1)

81-81: The change from an implicit to an explicit variable declaration (var enc int) is a minor stylistic adjustment that improves the clarity of the variable's type. This is generally a good practice for readability, especially in complex functions. However, ensure that such changes align with the project's coding standards and consider if a rationale should be provided for stylistic changes, especially if part of a larger effort to standardize code style.

message/pool/message_test.go (2)

358-358: Changing the assertion order to require.Equal(t, "seek error", err.Error()) aligns with best practices by placing the expected value first. This improves the readability of test failures and is a good practice to follow in test assertions.


389-389: The change to use require.Len(t, data, n) is an improvement, as it leverages a more specific assertion method designed for checking the length of collections. This approach enhances the clarity of test failures by providing more informative error messages.

udp/server/server.go (1)

121-121: Using errors.New for creating simple error messages without formatting is a good practice for consistency and clarity in error handling.

message/options_test.go (2)

267-267: Replacing require.Equal(t, nil, err) with require.NoError(t, err) in test assertions is a good practice for clarity and idiomatic Go code.


291-291: The use of require.NoError(t, err) for error assertions in tests is idiomatic and improves the readability of the test code.

net/connUDP_internal_test.go (3)

93-95: The replacement of assert with require for error handling in tests is a good practice, ensuring immediate failure upon encountering an error. This change enhances the robustness of the test suite.


211-211: Consider using require instead of assert for payload comparison to maintain consistency and ensure that the test fails immediately if the payloads do not match.


227-230: The replacement of assert with require for error handling in tests is consistent with best practices, ensuring immediate failure upon encountering an error. This enhances the robustness of the test suite.

tcp/client/conn.go (1)

169-169: Using errors.New for static error messages, as done here, is a good practice for consistency and clarity in error handling.

message/option.go (2)

115-115: Replacing fmt.Errorf with errors.New for static error messages, as done here, aligns with Go's best practices for error handling.


236-236: Using errors.New for static error messages, as done here, is consistent with Go's best practices for error handling.

udp/server_test.go (2)

200-200: The change from a length check to require.NotEmpty for the got variable in the TestServerDiscover function is a positive improvement. It makes the intention of the test clearer and leverages the require package's functionality for more expressive tests.


197-203: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-250]

Overall, the test suite in udp/server_test.go is well-structured and follows good practices in Go testing, including the use of require for assertions and proper resource cleanup. The change to use require.NotEmpty enhances the clarity of the test's intention. Keep up the good work!

net/connUDP.go (4)

248-248: The use of errors.New for creating error messages without formatting is a good practice for consistency and performance.


398-398: Using errors.New for simple error messages is appropriate and aligns with Go best practices.


472-472: The transition to errors.New for creating error messages is correctly applied here, enhancing error handling consistency.


611-611: Correctly using errors.New for error creation without formatting parameters. This change improves the readability and performance of error handling.

udp/client_test.go (11)

108-108: Replacing assert.True with require.Equal for checking message types enhances test clarity and failure handling.


116-116: Using assert.Equal for message type comparison is more direct and readable than the previous approach.


124-124: Correctly using assert.NotEqual for message type assertion improves the specificity of the test.


134-134: Switching to require.Equal for message type checks in tests ensures immediate failure on mismatch, which is a good practice.


210-210: The use of assert.Equal for message type assertion in a separate message test case is appropriate and improves test accuracy.


342-342: Using assert.Equal for message type checks in test cases is a good practice for ensuring the expected behavior.


356-356: Correctly using assert.Equal for message type assertion in test cases improves the clarity and accuracy of the tests.


472-472: The use of assert.Equal for asserting message types in test cases is appropriate and enhances test clarity.


486-486: Applying assert.Equal for message type checks in test cases ensures that the tests are specific and clear.


585-585: Using assert.Equal for message type assertion in delete operation tests is a good practice for ensuring the expected behavior.


593-593: Correctly using assert.Equal for message type checks in test cases improves the clarity and accuracy of the tests.

tcp/client_test.go (1)

840-840: The change from require.True to require.ErrorIs for the error assertion enhances the test's specificity and robustness by ensuring the exact error is checked. This aligns well with the PR's objective of improving error handling.

net/blockwise/blockwise_test.go (3)

305-305: Changing from assert.Error to require.Error for error assertions ensures that tests fail immediately upon encountering an error, aligning with the PR's objective to enhance testing methodologies.


309-309: Changing from require.Len to require.NotEmpty for cache length checks focuses the assertion on the essential aspect of non-emptiness, which is more relevant to the test's objectives and enhances flexibility.


311-311: Consistently changing from require.Len to require.NotEmpty for cache length checks across different parts of the file enhances the focus on the essential aspect of non-emptiness, aligning with the PR's objectives.

dtls/client_test.go (1)

211-211: The error handling in the TestConnGetSeparateMessage function has been changed from assert to require, ensuring that the test fails immediately if an unexpected message is received. This change improves the robustness of the test by halting execution on failure, which is especially important in asynchronous or network-dependent tests where continued execution after a failure could lead to misleading results or further errors.

udp/client/conn_test.go (2)

377-377: The error handling in the TestConnGetSeparateMessage function has been changed from assert to require, ensuring that the test fails immediately if an unexpected message is received. This change improves the robustness of the test by halting execution on failure, which is especially important in asynchronous or network-dependent tests where continued execution after a failure could lead to misleading results or further errors.


1006-1006: The change from require.True to require.ErrorIs for error comparison in the TestConnRequestMonitorDropRequest function is a significant improvement. It allows for more precise error handling by verifying that the error is exactly what is expected, rather than just checking for the presence of an error. This makes the test more accurate and reliable.

net/blockwise/blockwise.go (9)

207-207: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


210-210: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


219-219: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


586-586: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


597-597: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


761-761: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


719-719: The use of errors.New for creating error messages without formatting is appropriate and improves the consistency of error handling in the codebase.


666-666: Consider handling the error from payloadFile.Truncate(payloadSize) immediately after its occurrence to ensure that any issues with truncating the file are caught and handled appropriately.


207-210: Overall, the replacement of fmt.Errorf with errors.New across various error handling scenarios in net/blockwise/blockwise.go is a positive change that standardizes error creation using the errors package. This aligns well with the PR's objectives of enhancing error handling consistency. No further issues were identified in the segments provided. The rest of the file should be reviewed with the same level of detail to ensure consistency and quality throughout.

Also applies to: 219-219, 586-586, 597-597, 761-761, 719-719

udp/client/conn.go (5)

326-326: The replacement of fmt.Errorf with errors.New for the error message related to token validation is appropriate since the error message does not require formatting. This change aligns with best practices for generating static error messages.


945-945: The replacement of fmt.Errorf with errors.New for the error message related to multicast message confirmability is correct, as the error message is static and does not require formatting parameters. This is a good practice for creating more efficient and readable error handling.


326-326: Consider adding more specific error messages to enhance debugging and error tracing capabilities. While errors.New("invalid token") is clear, it might be beneficial to include why the token is considered invalid if possible.


945-945: For multicast messages, the error message errors.New("multicast messages cannot be confirmable") is clear and informative. However, consider documenting this constraint in the function's comments to improve code readability and maintainability.


323-329: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-949]

Overall, the file demonstrates good adherence to Go best practices, including proper error handling, use of atomic operations for concurrency safety, and clear separation of concerns. However, consider the following suggestions to further improve the code:

  1. Error Handling Consistency: Ensure that all error messages are consistently informative across the entire file. This aids in debugging and maintenance.
  2. Documentation: Adding comments to public types and functions would enhance code readability and maintainability, especially for complex logic or when specific behavior is not immediately obvious from the code itself.
  3. Performance Considerations: For functions that are called frequently or are critical to the application's performance, evaluate the potential impact of operations within them. For example, the use of locks and atomic operations should be justified by the need for concurrency safety, and their performance impact should be considered.

udp/client_test.go Outdated Show resolved Hide resolved
pkg/sync/map.go Outdated Show resolved Hide resolved
mux/router.go Outdated Show resolved Hide resolved
Direct:
github.com/pion/dtls/v2 v2.2.8-0.20240327211025-8244c4570c01
github.com/pion/transport/v3 v3.0.2
golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8
golang.org/x/net v0.22.0

Indirect:
golang.org/x/crypto v0.21.0
golang.org/x/sys v0.18.0
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ce5dcc5 and 22afe59.
Files ignored due to path filters (2)
  • go.mod is excluded by !**/*.mod
  • go.sum is excluded by !**/*.sum
Files selected for processing (40)
  • .golangci.yml (5 hunks)
  • dtls/client_test.go (1 hunks)
  • dtls/server/server.go (2 hunks)
  • dtls/server/session.go (2 hunks)
  • message/codes/code_string.go (2 hunks)
  • message/codes/codes.go (2 hunks)
  • message/codes/codes_test.go (1 hunks)
  • message/option.go (3 hunks)
  • message/options_test.go (2 hunks)
  • message/pool/message_test.go (2 hunks)
  • message/status/status_test.go (3 hunks)
  • mux/router.go (1 hunks)
  • mux/router_test.go (1 hunks)
  • net/blockwise/blockwise.go (9 hunks)
  • net/blockwise/blockwise_test.go (7 hunks)
  • net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
  • net/connUDP.go (6 hunks)
  • net/connUDP_internal_test.go (3 hunks)
  • net/conn_test.go (3 hunks)
  • net/dtlslistener.go (1 hunks)
  • net/observation/handler.go (4 hunks)
  • net/tlslistener_test.go (4 hunks)
  • pkg/cache/cache_test.go (2 hunks)
  • pkg/sync/map.go (1 hunks)
  • server.go (2 hunks)
  • tcp/client/conn.go (1 hunks)
  • tcp/client/session.go (2 hunks)
  • tcp/client_test.go (1 hunks)
  • tcp/coder/bench_test.go (1 hunks)
  • tcp/coder/coder_test.go (1 hunks)
  • tcp/server/server.go (2 hunks)
  • udp/client/conn.go (2 hunks)
  • udp/client/conn_test.go (3 hunks)
  • udp/client_test.go (11 hunks)
  • udp/coder/bench_test.go (1 hunks)
  • udp/coder/coder_test.go (1 hunks)
  • udp/server/discover.go (3 hunks)
  • udp/server/server.go (4 hunks)
  • udp/server/session.go (2 hunks)
  • udp/server_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (39)
  • .golangci.yml
  • dtls/client_test.go
  • dtls/server/server.go
  • dtls/server/session.go
  • message/codes/code_string.go
  • message/codes/codes.go
  • message/codes/codes_test.go
  • message/option.go
  • message/options_test.go
  • message/pool/message_test.go
  • message/status/status_test.go
  • mux/router.go
  • mux/router_test.go
  • net/blockwise/blockwise.go
  • net/blockwise/blockwise_test.go
  • net/client/limitParallelRequests/limitParallelRequests_test.go
  • net/connUDP.go
  • net/connUDP_internal_test.go
  • net/conn_test.go
  • net/dtlslistener.go
  • net/observation/handler.go
  • pkg/cache/cache_test.go
  • pkg/sync/map.go
  • server.go
  • tcp/client/conn.go
  • tcp/client/session.go
  • tcp/client_test.go
  • tcp/coder/bench_test.go
  • tcp/coder/coder_test.go
  • tcp/server/server.go
  • udp/client/conn.go
  • udp/client/conn_test.go
  • udp/client_test.go
  • udp/coder/bench_test.go
  • udp/coder/coder_test.go
  • udp/server/discover.go
  • udp/server/server.go
  • udp/server/session.go
  • udp/server_test.go
Additional comments not posted (1)
net/tlslistener_test.go (1)

78-78: The replacement of assert with require in test assertions is a positive change, ensuring that tests fail immediately on error, which aligns with the PR's objectives to enhance error handling and testing methodologies. This change improves test reliability by enforcing critical error handling.

Also applies to: 81-81, 89-89, 112-112, 115-115, 123-123, 128-128, 130-130, 164-164, 167-167, 175-175, 213-213, 220-220, 223-223

Copy link

sonarcloud bot commented Apr 3, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
25.0% Coverage on New Code (required ≥ 65%)

See analysis details on SonarCloud

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 25.00000% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 75.86%. Comparing base (ce5dcc5) to head (5f64797).

Files Patch % Lines
net/blockwise/blockwise.go 25.00% 6 Missing ⚠️
net/connUDP.go 25.00% 3 Missing ⚠️
udp/server/discover.go 0.00% 3 Missing ⚠️
udp/server/server.go 0.00% 3 Missing ⚠️
dtls/server/server.go 0.00% 2 Missing ⚠️
net/observation/handler.go 0.00% 2 Missing ⚠️
tcp/server/server.go 0.00% 2 Missing ⚠️
udp/client/conn.go 0.00% 2 Missing ⚠️
message/codes/code_string.go 0.00% 1 Missing ⚠️
net/dtlslistener.go 0.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #545      +/-   ##
==========================================
+ Coverage   75.81%   75.86%   +0.05%     
==========================================
  Files          72       72              
  Lines        5945     5941       -4     
==========================================
  Hits         4507     4507              
+ Misses       1044     1042       -2     
+ Partials      394      392       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ce5dcc5 and 5f64797.
Files ignored due to path filters (2)
  • go.mod is excluded by !**/*.mod
  • go.sum is excluded by !**/*.sum
Files selected for processing (40)
  • .golangci.yml (5 hunks)
  • dtls/client_test.go (1 hunks)
  • dtls/server/server.go (2 hunks)
  • dtls/server/session.go (2 hunks)
  • message/codes/code_string.go (2 hunks)
  • message/codes/codes.go (2 hunks)
  • message/codes/codes_test.go (1 hunks)
  • message/option.go (3 hunks)
  • message/options_test.go (2 hunks)
  • message/pool/message_test.go (2 hunks)
  • message/status/status_test.go (3 hunks)
  • mux/router.go (1 hunks)
  • mux/router_test.go (1 hunks)
  • net/blockwise/blockwise.go (9 hunks)
  • net/blockwise/blockwise_test.go (7 hunks)
  • net/client/limitParallelRequests/limitParallelRequests_test.go (2 hunks)
  • net/connUDP.go (6 hunks)
  • net/connUDP_internal_test.go (3 hunks)
  • net/conn_test.go (3 hunks)
  • net/dtlslistener.go (1 hunks)
  • net/observation/handler.go (4 hunks)
  • net/tlslistener_test.go (4 hunks)
  • pkg/cache/cache_test.go (2 hunks)
  • pkg/sync/map.go (1 hunks)
  • server.go (2 hunks)
  • tcp/client/conn.go (1 hunks)
  • tcp/client/session.go (2 hunks)
  • tcp/client_test.go (1 hunks)
  • tcp/coder/bench_test.go (1 hunks)
  • tcp/coder/coder_test.go (1 hunks)
  • tcp/server/server.go (2 hunks)
  • udp/client/conn.go (2 hunks)
  • udp/client/conn_test.go (3 hunks)
  • udp/client_test.go (11 hunks)
  • udp/coder/bench_test.go (1 hunks)
  • udp/coder/coder_test.go (1 hunks)
  • udp/server/discover.go (3 hunks)
  • udp/server/server.go (4 hunks)
  • udp/server/session.go (2 hunks)
  • udp/server_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (39)
  • .golangci.yml
  • dtls/client_test.go
  • dtls/server/server.go
  • dtls/server/session.go
  • message/codes/code_string.go
  • message/codes/codes.go
  • message/codes/codes_test.go
  • message/option.go
  • message/options_test.go
  • message/pool/message_test.go
  • message/status/status_test.go
  • mux/router.go
  • mux/router_test.go
  • net/blockwise/blockwise.go
  • net/blockwise/blockwise_test.go
  • net/client/limitParallelRequests/limitParallelRequests_test.go
  • net/connUDP.go
  • net/connUDP_internal_test.go
  • net/conn_test.go
  • net/dtlslistener.go
  • net/observation/handler.go
  • pkg/cache/cache_test.go
  • pkg/sync/map.go
  • server.go
  • tcp/client/conn.go
  • tcp/client/session.go
  • tcp/client_test.go
  • tcp/coder/bench_test.go
  • tcp/coder/coder_test.go
  • tcp/server/server.go
  • udp/client/conn.go
  • udp/client/conn_test.go
  • udp/client_test.go
  • udp/coder/bench_test.go
  • udp/coder/coder_test.go
  • udp/server/discover.go
  • udp/server/server.go
  • udp/server/session.go
  • udp/server_test.go
Additional comments not posted (2)
net/tlslistener_test.go (2)

109-133: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [78-130]

The replacement of assert with require for error handling in tests is a good practice to ensure immediate test failure on critical errors.


161-178: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [164-223]

The replacement of assert with require in this test function is consistent with the PR's objective to improve error handling in tests.

net/tlslistener_test.go Show resolved Hide resolved
@Danielius1922 Danielius1922 merged commit 6ed2ef2 into master Apr 3, 2024
13 of 14 checks passed
@Danielius1922 Danielius1922 deleted the adam/feature/update-deps branch April 3, 2024 06:43
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.

2 participants