Skip to content
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

HTTP1ConnectionsProvider.state needs to become private #234

Closed
weissi opened this issue May 29, 2020 · 1 comment
Closed

HTTP1ConnectionsProvider.state needs to become private #234

weissi opened this issue May 29, 2020 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@weissi
Copy link
Contributor

weissi commented May 29, 2020

Right now, HTTP1ConnectionsProvider.state is internal and some tests modify the state without HTTP1ConnectionsProvider knowing about this.

That cannot be right because HTTP1ConnectionsProvider cannot clean up stuff it didn't know happened. Therefore we see extremely fragile and ugly cleanup code in some tests.

@weissi
Copy link
Contributor Author

weissi commented May 29, 2020

when this is fixed, HTTP1ConnectionProvider.closePromise must become private too, there's no reason for the tests to fulfil internal promises of the provider.

@artemredkin artemredkin added the enhancement New feature or request label Jun 13, 2020
@artemredkin artemredkin added this to the 1.2.1 milestone Jun 13, 2020
@artemredkin artemredkin modified the milestones: 1.2.1, 1.2.2 Aug 20, 2020
ya-pulser added a commit to ya-pulser/async-http-client that referenced this issue Sep 28, 2020
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
ya-pulser added a commit to ya-pulser/async-http-client that referenced this issue Sep 28, 2020
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.
ya-pulser added a commit to ya-pulser/async-http-client that referenced this issue Sep 28, 2020
…e so modification

allowed only using exposed API.

Motivation:

Having a setter for internal state of ConnectionsState led to a subset
of test testing invalid invariants, for example when we have at the same
time an available connecion and a waiter, which is an invalid state of
the system.

Modifications:

* test of ConnectionsState
* ConnectionsState
* ConnectionPool

are modified so the state under tests is achieved only by a sequence of
modifications invoked by state API.

During modification some tests are eliminated as they were testing
artificial state, which can not be achieved by exposed APIs.

ConnectionsState is pruned from "replace" as in no valid state we can
have a situation when we can "replace" a connection.

Invalid invariants and tests are removed.

Result:

We do not have a way to modify state of the ConnectionsState by direct
interaction with private state of the object. Getter on the state is
considered harmless and used for tests only.
ya-pulser added a commit to ya-pulser/async-http-client that referenced this issue Sep 28, 2020
…e so modification

allowed only using exposed API.

Motivation:

Having a setter for internal state of ConnectionsState led to a subset
of test testing invalid invariants, for example when we have at the same
time an available connecion and a waiter, which is an invalid state of
the system.

Modifications:

* test of ConnectionsState
* ConnectionsState
* ConnectionPool

are modified so the state under tests is achieved only by a sequence of
modifications invoked by state API.

During modification some tests are eliminated as they were testing
artificial state, which can not be achieved by exposed APIs.

ConnectionsState is pruned from "replace" as in no valid state we can
have a situation when we can "replace" a connection.

Invalid invariants and tests are removed.

Result:

We do not have a way to modify state of the ConnectionsState by direct
interaction with private state of the object. Getter on the state is
considered harmless and used for tests only.
Lukasa pushed a commit that referenced this issue Sep 30, 2020
…r into internal state (#310)

* Introduce helper methods for test of ConnectionsState

Motivation:

Issue #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

* Move Connection tests out of ConnectionsState tests into separate file.

Motivation:

Clean up of code to address issue #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.

* Gather Connection code into Connection.swift

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.

* Introduce generic type ConnectionType into ConnectionsState

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 added a commit to ya-pulser/async-http-client that referenced this issue Oct 1, 2020
…e so modification

allowed only using exposed API.

Motivation:

Having a setter for internal state of ConnectionsState led to a subset
of test testing invalid invariants, for example when we have at the same
time an available connecion and a waiter, which is an invalid state of
the system.

Modifications:

* test of ConnectionsState
* ConnectionsState
* ConnectionPool

are modified so the state under tests is achieved only by a sequence of
modifications invoked by state API.

During modification some tests are eliminated as they were testing
artificial state, which can not be achieved by exposed APIs.

ConnectionsState is pruned from "replace" as in no valid state we can
have a situation when we can "replace" a connection.

Invalid invariants and tests are removed.

Result:

We do not have a way to modify state of the ConnectionsState by direct
interaction with private state of the object. Getter on the state is
considered harmless and used for tests only.
ya-pulser added a commit to ya-pulser/async-http-client that referenced this issue Oct 2, 2020
…e so modification

allowed only using exposed API.

Motivation:

Having a setter for internal state of ConnectionsState led to a subset
of test testing invalid invariants, for example when we have at the same
time an available connecion and a waiter, which is an invalid state of
the system.

Modifications:

* test of ConnectionsState
* ConnectionsState
* ConnectionPool

are modified so the state under tests is achieved only by a sequence of
modifications invoked by state API.

During modification some tests are eliminated as they were testing
artificial state, which can not be achieved by exposed APIs.

ConnectionsState is pruned from "replace" as in no valid state we can
have a situation when we can "replace" a connection.

Invalid invariants and tests are removed.

Result:

We do not have a way to modify state of the ConnectionsState by direct
interaction with private state of the object. Getter on the state is
considered harmless and used for tests only.
@Lukasa Lukasa closed this as completed in e401a28 Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants