Skip to content

`devshard`: It would be worth adding small tests asserting `pool.Config().MaxConns` for both payload storage constructors. `ConfigureMaxConns` itself is well tested, but the current tests wouldn’t catch the helper being accidentally removed from either constructor. #1851

Closed @tcharchian opened 2026-09-25 16:19 UTC 2 comments Updated 2026-10-15 00:28 UTC

Looks good overall, approved. It would be worth adding small tests asserting pool.Config().MaxConns for both payload storage constructors. ConfigureMaxConns itself is well tested, but the current tests wouldn’t catch the helper being accidentally removed from either constructor.

Originally posted by @aikuznetsov in https://github.com/gonka-ai/gonka/pull/1840#pullrequestreview-5312588423

💬 Comments (2)

@zpoken commented 2026-09-30 14:48 UTC

Added in #1883 : tests for both payload constructors, newPostgresStorage (common/storage/payloads) and NewPostgresStorage (decentralized-api/payloadstorage).

  • An invalid PG_POOL_MAX_CONNS must fail the constructor with the helper's error before any connection is made. Runs in -short, no Docker.
  • testcontainers: pool.Config().MaxConns is the default when the variable is unset and 3 when it is set. 3 is below pgx's floor of 4, so the check does not depend on the CPU count.

Verified by removing the ConfigureMaxConns call from both constructors: every new test fails, on hosts pinned to 3, 4 and 8 CPUs.

@akup

@a-kuprin commented 2026-10-15 00:28 UTC

Closed with https://github.com/gonka-ai/gonka/pull/1883

🔄 Auto-synced from Issue #1851 every hour.