Respond to sonarcloud maint issues#156
Merged
Merged
Conversation
Extract constants, add comments about intent in empty methods, set cognitive complexity limits for special files
Contributor
There was a problem hiding this comment.
Pull request overview
This PR primarily addresses SonarCloud maintenance/quality concerns by expanding unit test coverage across sshlib and protocol, refactoring repeated string literals into constants, and centralizing SonarCloud configuration at the root project level.
Changes:
- Added extensive new unit tests for SSH client/auth/channel flows, SFTP packet/dispatcher/client behavior, forwarding interfaces, and crypto utilities.
- Refactored several repeated literals (algorithm names, error messages, protocol strings) into constants to improve maintainability and reduce Sonar findings.
- Moved SonarCloud Gradle configuration from
:sshlibto the root project, updated CI invocation, and enabled coverage for:protocol.
Reviewed changes
Copilot reviewed 40 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sshlib/src/main/kotlin/org/connectbot/sshlib/SshClient.kt | Adds injectable constructor params and shared constants; expands wrapper behavior tested by new unit tests. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/HostKeyVerifier.kt | Expands default no-op interface methods into block bodies with clarifying comments. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/AuthHandler.kt | Expands default no-op interface methods into block bodies with clarifying comments. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PrivateKeyReader.kt | Replaces OpenSSH PEM markers with constants. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Base64Compat.kt | Unwraps InvocationTargetException on Android-reflection decode path. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketCipher.kt | Adds explicit default destroy() body with comment. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketMac.kt | Adds explicit default destroy() body with comment. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketAead.kt | Adds explicit default destroy() body with comment. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyTypes.kt | Introduces constant for "ssh-ed25519" to reduce duplication. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyEncryption.kt | Introduces cipher transformation constants to reduce duplication. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyDecryption.kt | Introduces cipher transformation constants to reduce duplication. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/JavaMlKemProvider.kt | Extracts reflective class/algorithm strings into constants. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/ed25519/Ed25519Provider.kt | Moves suppression to file-level and clarifies comment. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt | Extracts repeated “group not set” error string into a constant. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Algorithms.kt | Extracts repeated hash and key-type strings into constants. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt | Extracts repeated protocol strings/errors into constants; minor hostbound handling refactor. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpPacketIO.kt | Introduces SftpPacketTransport interface and makes SftpPacketIO implement it. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpDispatcher.kt | Accepts SftpPacketTransport to improve testability. |
| sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImpl.kt | Adds overload to inject packet transport for handshake tests. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/SshClientTest.kt | Adds extensive coverage for wrapper methods, forwarding helpers, auth state, and error mapping. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/SftpClientTest.kt | Adds coverage for listdir batching and handle-close behavior. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/ForwarderInterfacesTest.kt | Verifies close() delegates to stop() for forwarder interfaces. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/transport/TransportDependenciesTest.kt | Adds a basic dependency/behavior check for default address resolution. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/PacketPrimitiveInterfacesTest.kt | Adds coverage for default interface behaviors on crypto primitives. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/KeyEncryptionTest.kt | Adds round-trip and rejection tests for PEM/OpenSSH encryption helpers. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/AdditionalCoverageTest.kt | Adds targeted tests for ML-KEM wrapping, AES-CTR validation, and key-type inference errors. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshConnectionFlowTest.kt | Adds protocol-level flow tests for auth, channels, forwarding, and disconnect handling using a fake server. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt | Extends fake server to parse/emit more message types needed by new flow tests. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/Socks5HandlerTest.kt | Adds negative-path handshake tests for version/method/command/address-type parsing. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpPacketIOTest.kt | Adds tests for SFTP packet framing, buffering, and write serialization. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpDispatcherTest.kt | Adds tests for request/response correlation and read-loop error handling. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImplTest.kt | Adds broad unit coverage for SFTP handshake and command mappings via injectable transport/session fakes. |
| sshlib/src/test/kotlin/org/connectbot/sshlib/blocking/BlockingSshClientTest.kt | Adds coverage for connect/auth wrappers, forwarding wrappers, and disconnect delegation. |
| sshlib/src/test/kotlin/android/util/Base64.kt | Adds JVM test stub for android.util.Base64 to exercise reflection path. |
| sshlib/build.gradle.kts | Removes Sonar plugin/config from module build. |
| protocol/src/test/kotlin/org/connectbot/sshlib/protocol/KaitaiUtilsTest.kt | Adds tests for Kaitai struct serialization helpers and size limits. |
| protocol/build.gradle.kts | Enables tests + Kover coverage configuration for the protocol module. |
| build.gradle.kts | Centralizes Sonar configuration at root and wires Kover for subprojects. |
| .github/workflows/ci.yml | Updates Sonar invocation to run from root (./gradlew sonar). |
| gradle.properties | Enables Gradle caching/parallelism and sets JVM args. |
| gradle/verification-metadata.xml | Adds Gradle dependency verification metadata. |
| .gitignore | Ignores Gradle dependency verification keyring/cache backup files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
34c293d to
ef2cb27
Compare
Just pushing us over 90% coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.