Skip to content

Code clean up to to fix issue #234 - ConnectionsState exposes a setter into internal state #310

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

Merged
merged 4 commits into from
Sep 30, 2020

Conversation

ya-pulser
Copy link
Contributor

No description provided.

Motivation:

Issue swift-server#234 highlights that we directly manipulate ConnectionsState and
this commit prepares tests to be refactored to manipulate the state
by exposed APIs instead.

Modifications:

* introduce helper methods in ConnectionPoolTestsSupport.swift

Result:

* no observable changes
Motivation:

Clean up of code to address issue swift-server#234 - here we move away
connection tests to separate files outside of ConnectionsState tests
so we will be able to work on the ConnectionsState in focussed mode.

Modifications:

Connection tests moved to separate files.

Result:

No observable changes.
@swift-server-bot
Copy link

Can one of the admins verify this patch?

4 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@artemredkin
Copy link
Collaborator

@swift-server-bot test this please

}

struct ConnectionsState {
struct ConnectionsState<ConnectionType: PoolManageableConnection> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

small comment from me: this generic is only needed to we can mock the Connection type in order to make sure that ConnectionState tests do not depend on HTTP1ConnectionProvider.

Motivation:

For tests we will need a simple version of Connection, so here I gather
Connection code in one place and will generify ConnectionsState on next
commit.

Modifications:

Code of Connection is moved from multiple files into single
Connections.swift.

Result:

All tests are passing, no observable behaviour change.
Motivation:

To rework tests of ConnectionsState we want to have a "simpler" version
of Connection to be used, therefore here we convert ConnectionsState to
support generic type ConnectionType. We will substitute current
Connection with a test version in follow up commit.

Modifications:

ConnectionsState is altered to work on generic type ConnectionType
instead of solid type Connection.

Users of ConnectionsState are modified to provide type Connection into
ConnectionType in this commit.

Result:

Test are passing, no observable behaviour change.
@ya-pulser ya-pulser force-pushed the issue_234_01_preparation branch from 796bf33 to 5cb8902 Compare September 28, 2020 14:18
@artemredkin
Copy link
Collaborator

@swift-server-bot test this please

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 30, 2020

@swift-server-bot add to whitelist

Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice, I'm entirely happy with this refactor, great job.

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Sep 30, 2020
@Lukasa Lukasa merged commit c65b2ae into swift-server:main Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants