Skip to content

Add URLEndpointListener QE tests and TLS support in iOS TestServer#384

Merged
barkha06 merged 24 commits into
mainfrom
ios-p2p-tls
Apr 18, 2026
Merged

Add URLEndpointListener QE tests and TLS support in iOS TestServer#384
barkha06 merged 24 commits into
mainfrom
ios-p2p-tls

Conversation

@barkha06
Copy link
Copy Markdown
Contributor

  • Added QE test coverage for URLEndpointListener, migrated from mobile-testkit to validate legacy peer-to-peer functionality
  • Implemented TLS support in iOS TestServer for URLEndpointListener
  • Enabled execution of peer-to-peer replication tests with TLS
  • Added corresponding spec files

Copy link
Copy Markdown
Contributor

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

Adds QE test coverage for Couchbase Lite peer-to-peer URLEndpointListener replication and introduces TLS support for the iOS TestServer listener endpoint.

Changes:

  • Added a new QE test suite covering multiple peer-to-peer replication topologies and resilience scenarios (incl. TLS).
  • Implemented TLS identity import/reuse for iOS URLEndpointListener startup.
  • Extended the Python client API to (a) send listener TLS identity material and (b) generate wss:// replication URLs when TLS is enabled.

Reviewed changes

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

Show a summary per file
File Description
tests/QE/test_peer_to_peer.py New QE peer-to-peer replication tests (incl. TLS + restart scenario).
spec/tests/QE/test_peer_to_peer.md Spec describing the new peer-to-peer QE test cases.
servers/ios/TestServer/Utils/DatabaseManager.swift Adds TLS identity import/reuse and configures tlsIdentity for the listener.
servers/ios/TestServer/Handlers/StartListener.swift Wires request fields (identity, reuseIdentity) into listener startup.
servers/ios/TestServer/ContentTypes/StartListenerRequest.swift Adds identity and reuseIdentity fields to the start-listener request type.
client/src/cbltest/v1/requests.py Adds identity and reuse_identity to /startListener request payload generation.
client/src/cbltest/api/testserver.py Adds tls flag to produce wss:// URLs for replication endpoints.
client/src/cbltest/api/listener.py Adds TLS identity generation, exposes identity, and passes identity/reuse flags to the server request.

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

Comment thread servers/ios/TestServer/ContentTypes/StartListenerRequest.swift Outdated
Comment thread servers/ios/TestServer/Utils/DatabaseManager.swift Outdated
Comment thread servers/ios/TestServer/Utils/DatabaseManager.swift Outdated
Comment thread client/src/cbltest/api/listener.py
Comment thread client/src/cbltest/v1/requests.py
Comment thread tests/QE/test_peer_to_peer.py Outdated
Comment thread tests/QE/test_peer_to_peer.py Outdated
Comment thread tests/QE/test_peer_to_peer.py Outdated
Comment thread tests/QE/test_peer_to_peer.py Outdated
Comment thread client/src/cbltest/api/testserver.py Outdated
barkha06 and others added 5 commits April 13, 2026 21:17
Copy link
Copy Markdown
Contributor

@pasin pasin left a comment

Choose a reason for hiding this comment

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

I have review the iOS TestServer code. Please see my comment.

Comment thread servers/ios/TestServer/ContentTypes/StartListenerRequest.swift Outdated
Comment thread servers/ios/TestServer/ContentTypes/StartListenerRequest.swift Outdated
Comment thread servers/ios/TestServer/Handlers/StartListener.swift Outdated
Comment thread servers/ios/TestServer/Utils/DatabaseManager.swift
Comment thread client/src/cbltest/api/listener.py Outdated
database: Database,
collections: list[str],
port: int | None = None,
port: int = 59840,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see a reason to make this have a default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a required parameter because the replication URL needs to be generated based on the port, so the ty lint check failed since it was marked optional but it needs to have int value later.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It will still have an int value after it is started. I think the ty lint isn't getting all of the information.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't get why the above is not enough to satisfy this situation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure, but here's the failure:
--> /Users/barkha.goyal/Desktop/temp2/couchbase-lite-tests/tests/QE/test_peer_to_peer.py:376:24 | 374 | all_dbs[1], 375 | endpoint=cblpytest.test_servers[0].replication_url( 376 | "db1", listener1.port, tls=True | ^^^^^^^^^^^^^^ Expected int, found Unknown | int | None 377 | ), 378 | replicator_type=replicator_type, | info: Element None of this union is not assignable to int info: Method defined here --> /Users/barkha.goyal/Desktop/temp2/couchbase-lite-tests/client/src/cbltest/api/testserver.py:146:9 | 144 | await self.__request_factory.send_request(self.__index, request) 145 | 146 | def replication_url(self, db_name: str, port: int, tls: bool = False): | ^^^^^^^^^^^^^^^ --------- Parameter declared here 147 | """

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One way to avoid would this is to make port a private parameter and define a getter for it, since the getter will always return an int, the ty lint would accept it.

Comment thread client/src/cbltest/api/listener.py Outdated
@barkha06 barkha06 self-assigned this Apr 14, 2026
@borrrden
Copy link
Copy Markdown
Member

Also, regardless since you've changed the REST API you need to modify the OpenAPI spec and bump the version for the iOS test server and TDK client.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026

Redocly preview

API

@barkha06 barkha06 requested review from borrrden and pasin April 16, 2026 14:48
Comment thread servers/ios/TestServer/Utils/DatabaseManager.swift Outdated
if let id = identity {
importedIdentity = try DatabaseManager.createTLSIdentity( for: id, label:label)
} else {
guard let existingIdentity = try TLSIdentity.identity(withLabel: label) else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let do not do this logic to avoid using any stale identity which may cause a test failure that is difficult to figure out. So if the created identity is nil, we can throw the error right away.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can do something like this

listenerConfig.disableTLS = disableTLS

if let id = identity {
    guard let tlsIdentity = try DatabaseManager.createTLSIdentity( for: id, label:label) else { 
        throw TestServerError.badRequest("Failed to import TLS identity")
    }
    listenerConfig.tlsIdentity = tlsIdentity
}

Copy link
Copy Markdown
Member

@borrrden borrrden Apr 16, 2026

Choose a reason for hiding this comment

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

This will fail if a replicator is created twice with the same label, as is done in one of the tests. I had requested something like this to indicate passing nil is the way to say "use the previous existing identity"

Copy link
Copy Markdown
Member

@borrrden borrrden left a comment

Choose a reason for hiding this comment

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

Also why did the default port situation get resolved? I don't see a reply to what I said. Currently the default is to let the system choose the port, and it will be set upon start.

Comment thread spec/api/api.yaml
@barkha06
Copy link
Copy Markdown
Contributor Author

Also why did the default port situation get resolved? I don't see a reply to what I said. Currently the default is to let the system choose the port, and it will be set upon start.

I might've done that by mistake, I apologise for the oversight

@barkha06 barkha06 requested a review from borrrden April 17, 2026 06:10
Comment thread spec/tests/QE/test_peer_to_peer.md Outdated
@barkha06 barkha06 merged commit e6c5afb into main Apr 18, 2026
8 checks passed
@barkha06 barkha06 deleted the ios-p2p-tls branch April 18, 2026 12:23
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.

4 participants