Add timeout for retrieving certificates in test

[sunkup]

#82 changed the implementation of getSiteCertificates.

The new getSiteCertificates implementation (now in the unit test) creates an SSLSocket and calls startHandshake() with no connect/read timeouts, so we get infinite run time in CI.

To fix this, this PR

• moves TestCertStore to separate file
• adds timeouts to the certificate retrieval

[rfc2822]

A timeout looks like a good idea, but a socket should have reasonable default timeouts (not infinite). Is this really the cause of the problem?

I see that in #82 the conn.inputStream.read() (which guarantees that the connection is established) of the HttpUrlConnection was replaced by sslSocket.startHandshake(). Do we need this change and can it be related? In the original code, we didn't have to create a socket manually.

Or in other words: can we change it back to conn.inputStream.read() and it works again?

[sunkup]

Or in other words: can we change it back to conn.inputStream.read() and it works again?

Yes, that should also work. I just wanted to get rid of the deprecations while at it. I think it should work like this, but would you like me to change it back?

[rfc2822]

Yes, that should also work. I just wanted to get rid of the deprecations while at it. I think it should work like this, but would you like me to change it back?

It just looks like a much easier solution that doesn't cause extra work (like thinking about sockets and timeouts), so instead of adding more code, yes, I'd prefer the single-line solution, especially because it's only a helper for testing. For production code I'd avoid HttpUrlConnection.

There shouldn't be a deprecation of read? Or do you mean the whole HttpUrlConnection which is deprecated? I think we can keep it in the test helper. Otherwise we can replace it by okhttp at some time if the UrlConnection is really dropped.

[Copilot]

Pull Request Overview

This PR addresses an infinite runtime issue in CI caused by a previous change to getSiteCertificates. The fix involves adding connect and read timeouts to certificate retrieval and refactoring the test code structure.

• Extracts TestCertStore class into a separate test file for better organization
• Replaces the SSLSocket-based certificate retrieval with HttpsURLConnection that includes 5-second timeouts
• Simplifies certificate retrieval logic while maintaining test functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
TestCertStore.kt	New file containing the extracted TestCertStore test implementation, previously embedded in CustomCertManagerTest.kt
CustomCertManagerTest.kt	Refactored getSiteCertificates method to use HttpsURLConnection with timeouts instead of raw SSLSocket; removed TestCertStore class and unused imports

---

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

[rfc2822]

Tests are failing, resetting to draft

[sunkup]

Sorry about that. I think calling getSiteCertificates() as soon as the class is loaded made it hang forever in CI only, because it worked just fine locally. I moved it to @BeforeClass in the companion object which is probably cleaner anyways.

[rfc2822]

Minor suggestions

[rfc2822]

Nice 👍🏻