Skip to content

Commit

Permalink
Preventing each acceptance test from spawning another provider
Browse files Browse the repository at this point in the history
  • Loading branch information
detro committed Oct 31, 2024
1 parent e9c8400 commit 3524afd
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 24 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ jobs:
image: zookeeper:${{ matrix.zookeeper }}
ports:
- 2181:2181
env:
ZOO_MAX_CLIENT_CNXNS: 200

steps:

Expand Down
12 changes: 6 additions & 6 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ func NewClientFromEnv() (*Client, error) {
return NewClient(zkServers, zkSessionInt, zkUsername, zkPassword)
}

// Close the Client underlying connection.
func (c *Client) Close() {
fmt.Println("[DEBUG] Closing underlying ZooKeeper connection")
c.zkConn.Close()
}

// Create a ZNode at the given path.
//
// Note that any necessary ZNode parents will be created if absent.
Expand Down Expand Up @@ -268,6 +262,12 @@ func (c *Client) Update(path string, data []byte, acl []zk.ACL) (*ZNode, error)
return c.Read(path)
}

// Close the Client underlying connection.
func (c *Client) Close() {
fmt.Println("[DEBUG] Closing underlying ZooKeeper connection")
c.zkConn.Close()
}

// Delete the given ZNode.
//
// Note that will also delete any child ZNode, recursively.
Expand Down
5 changes: 3 additions & 2 deletions internal/client/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ func NewPool() *Pool {
}
}

// GetClient retrieves (or creates) a Client.
func (p *Pool) GetClient(servers string, sessionTimeoutSec int, username string, password string) (*Client, error) {
// GetOrCreateClient retrieves (or creates) a Client.
// A new client is created for each unique set of construction parameters.
func (p *Pool) GetOrCreateClient(servers string, sessionTimeoutSec int, username string, password string) (*Client, error) {
p.mu.Lock()
defer p.mu.Unlock()

Expand Down
5 changes: 3 additions & 2 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
)

func New() (*schema.Provider, error) {
// Hold a pool of Client, so a Client connecting to the same ZooKeeper can be shared between resources.
// Hold a Clients Pool, so connection to the same ZooKeeper can be shared between resources.
// The client is safe for concurrent usage.
clientPool := client.NewPool()

return &schema.Provider{
Expand Down Expand Up @@ -65,7 +66,7 @@ func New() (*schema.Provider, error) {
if servers != "" {
// NOTE: Client Pool above is in a closure here
// because we don't have a way to add fields to the Provider.
c, err := clientPool.GetClient(servers, sessionTimeout, username, password)
c, err := clientPool.GetOrCreateClient(servers, sessionTimeout, username, password)

if err != nil {
// Report inability to connect internal Client
Expand Down
26 changes: 14 additions & 12 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,26 @@ import (
func TestProvider(t *testing.T) {
assert := testifyAssert.New(t)

provider, err := provider.New()
p, err := provider.New()
assert.NoError(err)

assert.NoError(provider.InternalValidate())
assert.NoError(p.InternalValidate())
}

// providerFactoriesMap associates to each Provider factory instance, a name.
//
// WARN: This is important as this will be the name the provider will be expected
// to have when executing the acceptance tests.
// Fail to match the provider expected name will mean that the underlying binary
// terraform, used during acceptance tests, will error complaining it can't find
// the provider and `terraform init` should be executed.
//nolint:unparam
func providerFactoriesMap() map[string]func() (*schema.Provider, error) {
// Instantiate the provider in advance...
p, err := provider.New()
if err != nil {
panic(fmt.Errorf("failed to instantiate provider: %w", err))
}

// ... then return it within the factory method.
// This avoids the tests creating a new provider (and new connections) every single time.
return map[string]func() (*schema.Provider, error){
"zookeeper": provider.New,
"zookeeper": func() (*schema.Provider, error) {
return p, nil
},
}
}

Expand All @@ -41,8 +45,6 @@ func checkPreconditions(t *testing.T) {
}
}

// getTestZKClient can be used during test to procure a client.Client.

// confirmAllZNodeDestroyed should be used with the field `CheckDestroy` of resource.TestCase.
func confirmAllZNodeDestroyed(s *terraform.State) error {
fmt.Println("[DEBUG] Confirming all ZNodes have been removed")
Expand Down

0 comments on commit 3524afd

Please sign in to comment.