Skip to content

Conversation

@sunkup
Copy link
Member

@sunkup sunkup commented Nov 5, 2025

Remove conscrypt but keep it in integration tests. Projects using cert4android should themselves decide whether they want to use conscrypt or not.

@sunkup sunkup self-assigned this Nov 5, 2025
@sunkup sunkup added the refactoring Quality improvement of existing functions label Nov 5, 2025
@sunkup sunkup linked an issue Nov 5, 2025 that may be closed by this pull request
@sunkup sunkup requested a review from Copilot November 5, 2025 14:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the Conscrypt dependency from the main library implementation and moves it to test scope only. The main changes replace the direct usage of Conscrypt's default trust manager with the standard Java TrustManagerFactory API for retrieving the system trust manager.

  • Replaced Conscrypt.getDefaultX509TrustManager() with standard TrustManagerFactory API
  • Moved Conscrypt dependency from implementation to androidTestImplementation
  • Reorganized test files to consolidate Conscrypt-related tests

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
CustomCertStore.kt Replaced Conscrypt-specific trust manager retrieval with standard Java TrustManagerFactory API
CustomCertManager.kt Removed Conscrypt initialization from companion object
OkhttpTest.kt Moved Conscrypt initialization to @Before method and removed Conscrypt-specific test
ConscryptTest.kt Deleted file (tests moved to ConscryptIntegrationTest.kt)
ConscryptIntegrationTest.kt New file consolidating Conscrypt-related tests from other files
ConscryptIntegration.kt New test utility file for initializing Conscrypt in tests
build.gradle.kts Moved Conscrypt from implementation to androidTestImplementation dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sunkup sunkup marked this pull request as ready for review November 5, 2025 14:11
@sunkup sunkup requested a review from a team as a code owner November 5, 2025 14:11
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@ArnyminerZ ArnyminerZ left a comment

Choose a reason for hiding this comment

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

Looks good. Feel free to merge :)

@sunkup sunkup merged commit b3160b0 into main Nov 6, 2025
11 checks passed
@sunkup sunkup deleted the 74-decouple-from-conscrypt-1 branch November 6, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Quality improvement of existing functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decouple from Conscrypt

3 participants