Sage-Bionetworks / synapsePythonClient

Programmatic interface to Synapse services for Python
https://www.synapse.org
Apache License 2.0
65 stars 67 forks source link

[SYNPY-1456] Flaky integration tests due to connection issues #1085

Closed BryanFauble closed 2 months ago

BryanFauble commented 2 months ago

Problem:

  1. The HTTP connection pool was not being reused in integration tests. As a result a typical single run would create and tear down almost 400 connections.
  2. When a connection error occurred retry logic would retry the HTTP request which would in most cases fail the test since Synapse would conflict with the data it already has. These errors kept showing up
    ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host
    ConnectionResetError: [Errno 104] Connection reset by peer
  3. Builds were taking place during both the PR creation and commits. This means after a PR is created any new commits caused 2 builds to be ran.

Solution:

  1. Upgrading to pytest-v7, and a newer pytest-asyncio
  2. Updating the context of all integration tests to run within a singular asyncio event loop which allows us to share an HTTP connection pool: https://pytest-asyncio.readthedocs.io/en/latest/how-to-guides/run_session_tests_in_same_loop.html
  3. Wrapping the start of the integration tests with a rerun of 3 retries per test that fails to allow for some level of recoverability
  4. Only running builds on commit. This prevents running the build pipeline twice.

Testing:

  1. Relying on successful runs of the CI pipeline Verified that fewer connections are attempted (~400 -> ~200): image
sonarcloud[bot] commented 2 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint