Skip to content

Commit

Permalink
fix: skip tests for ConnectionPoolImpl if the env vars are not set.
Browse files Browse the repository at this point in the history
* Add a checkEnv function used to skip the test if the env vars are not set.

* Remove unneeded nil checks for HTTP clients cache

* Check if the WS Url cache is null before adding WS clients to the cache.
  • Loading branch information
Hartick committed Aug 28, 2024
1 parent 502165c commit ed0f235
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
15 changes: 11 additions & 4 deletions client/eth/connection_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,21 @@ func (c *ConnectionPoolImpl) Dial(string) error {
}

func (c *ConnectionPoolImpl) DialContext(ctx context.Context, _ string) error {
// NOTE: Check the cache for the HTTP URL is not needed because it
// is guaranteed to be non-nil when a ConnectionPoolImpl is created.
for _, url := range c.config.EthHTTPURLs {
client := NewHealthCheckedClient(c.config.HealthCheckInterval, c.logger)
if err := client.DialContextWithTimeout(ctx, url, c.config.DefaultTimeout); err != nil {
return err
}
c.cache.Add(url, client)
}

// Check is needed because the WS URL is optional.
if c.wsCache == nil {
return nil
}

for _, url := range c.config.EthWSURLs {
client := NewHealthCheckedClient(c.config.HealthCheckInterval, c.logger)
if err := client.DialContextWithTimeout(ctx, url, c.config.DefaultTimeout); err != nil {
Expand All @@ -119,13 +127,12 @@ func (c *ConnectionPoolImpl) DialContext(ctx context.Context, _ string) error {
return nil
}

// NOTE: this function assumes the cache is non-nil
// because it is guaranteed to be non-nil when a ConnectionPoolImpl is created.
func (c *ConnectionPoolImpl) GetHTTP() (Client, bool) {
c.mutex.Lock()
defer c.mutex.Unlock()

if c.cache == nil {
return nil, false
}
return c.getClientFrom(c.cache)
}

Expand All @@ -140,7 +147,7 @@ func (c *ConnectionPoolImpl) GetWS() (Client, bool) {
return c.getClientFrom(c.wsCache)
}

// NOTE: this function assumes the lock is held and cache is non-nil
// NOTE: this function assumes the lock is held and cache is non-nil.
func (c *ConnectionPoolImpl) getClientFrom(
cache *lru.Cache[string, *HealthCheckedClient],
) (Client, bool) {
Expand Down
38 changes: 30 additions & 8 deletions client/eth/connection_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,42 @@ var (
WSURL = os.Getenv("ETH_WS_URL")
)

// InitConnectionPool initializes a new connection pool.
func InitConnectionPool(
/******************************* HELPER FUNCTIONS ***************************************/

// NOTE: requires chain rpc url at env var `ETH_HTTP_URL` and `ETH_WS_URL`.
func checkEnv(t *testing.T) {
ethHTTPRPC := os.Getenv("ETH_HTTP_URL")
ethWSRPC := os.Getenv("ETH_WS_URL")
if ethHTTPRPC == "" || ethWSRPC == "" {
t.Skipf("Skipping test: no eth rpc url provided")
}
}

// initConnectionPool initializes a new connection pool.
func initConnectionPool(
cfg eth.ConnectionPoolConfig, writer io.Writer,
) (eth.ConnectionPool, error) {
logger := log.NewLogger(writer, "test-runner")
return eth.NewConnectionPoolImpl(cfg, logger)
}

// Use Init function as a setup function for the tests.
// It means each test will have to call Init function to set up the test.
func Init(
cfg eth.ConnectionPoolConfig, writer io.Writer, t *testing.T,
) (eth.ConnectionPool, error) {
checkEnv(t)
return initConnectionPool(cfg, writer)
}

/******************************* TEST CASES ***************************************/

// TestNewConnectionPoolImpl_MissingURLs tests the case when the URLs are missing.
func TestNewConnectionPoolImpl_MissingURLs(t *testing.T) {
cfg := eth.ConnectionPoolConfig{}
var logBuffer bytes.Buffer

_, err := InitConnectionPool(cfg, &logBuffer)
_, err := Init(cfg, &logBuffer, t)
require.ErrorContains(t, err, "ConnectionPool: missing URL for HTTP clients")
}

Expand All @@ -39,7 +61,7 @@ func TestNewConnectionPoolImpl_MissingWSURLs(t *testing.T) {
EthHTTPURLs: []string{HTTPURL},
}
var logBuffer bytes.Buffer
pool, err := InitConnectionPool(cfg, &logBuffer)
pool, err := Init(cfg, &logBuffer, t)

require.NoError(t, err)
require.NotNil(t, pool)
Expand All @@ -54,7 +76,7 @@ func TestNewConnectionPoolImpl(t *testing.T) {
EthWSURLs: []string{WSURL},
}
var logBuffer bytes.Buffer
pool, err := InitConnectionPool(cfg, &logBuffer)
pool, err := Init(cfg, &logBuffer, t)

require.NoError(t, err)
require.NotNil(t, pool)
Expand All @@ -68,7 +90,7 @@ func TestGetHTTP(t *testing.T) {
EthHTTPURLs: []string{HTTPURL},
}
var logBuffer bytes.Buffer
pool, _ := InitConnectionPool(cfg, &logBuffer)
pool, _ := Init(cfg, &logBuffer, t)
err := pool.Dial("")
require.NoError(t, err)

Expand All @@ -85,7 +107,7 @@ func TestGetWS(t *testing.T) {
EthWSURLs: []string{WSURL},
}
var logBuffer bytes.Buffer
pool, _ := InitConnectionPool(cfg, &logBuffer)
pool, _ := Init(cfg, &logBuffer, t)
err := pool.Dial("")

require.NoError(t, err)
Expand All @@ -102,7 +124,7 @@ func TestGetWS_WhenItIsNotSet(t *testing.T) {
EthHTTPURLs: []string{HTTPURL},
}
var logBuffer bytes.Buffer
pool, _ := InitConnectionPool(cfg, &logBuffer)
pool, _ := Init(cfg, &logBuffer, t)
err := pool.Dial("")
require.NoError(t, err)

Expand Down

0 comments on commit ed0f235

Please sign in to comment.