Skip to content
This repository has been archived by the owner on May 2, 2024. It is now read-only.

Commit

Permalink
Merge pull request #53 from ubuntu/removing-nss-testopts
Browse files Browse the repository at this point in the history 
Removing nss testopts
  • Loading branch information
@denisonbarbosa
denisonbarbosa committed Aug 31, 2022
2 parents c1238ac + 875512c commit a60deb5
Show file tree
Hide file tree
Showing 17 changed files with 69 additions and 131 deletions.
20 changes: 6 additions & 14 deletions internal/nss/group/group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,8 @@ type Group struct {
members []string /* Members of the group */
}

var testopts = []cache.Option{}

// setCacheOption set opts everytime we open a cache.
// This is not compatible with parallel testing as it needs to change a global state.
func setCacheOption(opts ...cache.Option) {
testopts = opts
}

// NewByName returns a passwd entry from a name.
func NewByName(ctx context.Context, name string) (g Group, err error) {
func NewByName(ctx context.Context, name string, cacheOpts ...cache.Option) (g Group, err error) {
defer func() {
if err != nil {
err = fmt.Errorf("failed to get group entry from name %q: %w", name, err)
Expand All @@ -41,7 +33,7 @@ func NewByName(ctx context.Context, name string) (g Group, err error) {
return Group{}, nss.ErrNotFoundENoEnt
}

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return Group{}, nss.ConvertErr(err)
}
Expand All @@ -61,7 +53,7 @@ func NewByName(ctx context.Context, name string) (g Group, err error) {
}

// NewByGID returns a group entry from a GID.
func NewByGID(ctx context.Context, gid uint) (g Group, err error) {
func NewByGID(ctx context.Context, gid uint, cacheOpts ...cache.Option) (g Group, err error) {
defer func() {
if err != nil {
err = fmt.Errorf("failed to get group entry from GID %d: %w", gid, err)
Expand All @@ -70,7 +62,7 @@ func NewByGID(ctx context.Context, gid uint) (g Group, err error) {

logger.Debug(ctx, "Requesting an group entry matching GID %d", gid)

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return Group{}, nss.ConvertErr(err)
}
Expand All @@ -93,12 +85,12 @@ var groupIterationCache *cache.Cache

// StartEntryIteration open a new cache for iteration.
// This needs to be called prior to calling NextEntry and be closed with EndEntryIteration.
func StartEntryIteration(ctx context.Context) error {
func StartEntryIteration(ctx context.Context, cacheOpts ...cache.Option) error {
if groupIterationCache != nil {
return nss.ConvertErr(errors.New("group entry iteration already in progress. End it before starting a new one"))
}

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return nss.ConvertErr(err)
}
Expand Down
28 changes: 15 additions & 13 deletions internal/nss/group/group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (

//nolint:dupl // TestNewByName and TestNewByGID have similar code that triggers dupl, despite being different.
func TestNewByName(t *testing.T) {
t.Parallel()

tests := map[string]struct {
name string
failingCache bool
Expand All @@ -30,6 +32,8 @@ func TestNewByName(t *testing.T) {
for name, tc := range tests {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()

cacheDir := t.TempDir()
testutils.PrepareDBsForTests(t, cacheDir, "users_in_db")

Expand All @@ -38,9 +42,8 @@ func TestNewByName(t *testing.T) {
if tc.failingCache {
opts = append(opts, cache.WithRootUID(4242))
}
group.SetCacheOption(opts...)

got, err := group.NewByName(context.Background(), tc.name)
got, err := group.NewByName(context.Background(), tc.name, opts...)
if tc.wantErrType != nil {
require.Error(t, err, "NewByName should have returned an error and hasn’t")
require.ErrorIs(t, err, tc.wantErrType, "NewByName has not returned expected error type")
Expand All @@ -56,6 +59,8 @@ func TestNewByName(t *testing.T) {

//nolint:dupl // TestNewByName and TestNewByGID have similar code that triggers dupl, despite being different.
func TestNewByGID(t *testing.T) {
t.Parallel()

tests := map[string]struct {
gid uint
failingCache bool
Expand All @@ -71,6 +76,8 @@ func TestNewByGID(t *testing.T) {
for name, tc := range tests {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()

cacheDir := t.TempDir()
testutils.PrepareDBsForTests(t, cacheDir, "users_in_db")

Expand All @@ -79,9 +86,8 @@ func TestNewByGID(t *testing.T) {
if tc.failingCache {
opts = append(opts, cache.WithRootUID(4242))
}
group.SetCacheOption(opts...)

got, err := group.NewByGID(context.Background(), tc.gid)
got, err := group.NewByGID(context.Background(), tc.gid, opts...)
if tc.wantErrType != nil {
require.Error(t, err, "NewByGID should have returned an error and hasn’t")
require.ErrorIs(t, err, tc.wantErrType, "NewByGID has not returned expected error type")
Expand Down Expand Up @@ -120,10 +126,9 @@ func TestNextEntry(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
group.SetCacheOption(opts...)

if !tc.noIterationInit {
err := group.StartEntryIteration(context.Background())
err := group.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "StartEntryIteration should succeed")
defer group.EndEntryIteration(context.Background())
}
Expand Down Expand Up @@ -176,21 +181,19 @@ func TestStartEndEntryIteration(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
group.SetCacheOption(opts...)

if tc.alreadyIterationInProgress {
err := group.StartEntryIteration(context.Background())
err := group.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "Setup: first startEntryIteration should have failed by hasn’t")
defer group.EndEntryIteration(context.Background())
}

if tc.cacheOpenError {
opts = append(opts, cache.WithRootUID(4242))
}
group.SetCacheOption(opts...)

if !tc.noStartIteration {
err := group.StartEntryIteration(context.Background())
err := group.StartEntryIteration(context.Background(), opts...)
if tc.wantStartIterationErr {
require.Error(t, err, "StartEntryIteration should have failed by hasn’t")
require.ErrorIs(t, err, nss.ErrUnavailableENoEnt, "Error should be of type Unavailable")
Expand All @@ -211,10 +214,9 @@ func TestRestartIterationWithoutEndingPreviousOne(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
group.SetCacheOption(opts...)

// First iteration group
err := group.StartEntryIteration(context.Background())
err := group.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "StartEntryIteration should succeed")
defer group.EndEntryIteration(context.Background()) // in case of an error in the middle of the test. No-op otherwise

Expand All @@ -226,7 +228,7 @@ func TestRestartIterationWithoutEndingPreviousOne(t *testing.T) {
require.NoError(t, err, "EndEntryIteration while iterating should work")

// Second iteration group
err = group.StartEntryIteration(context.Background())
err = group.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "restart a second entry iteration should succeed")
defer group.EndEntryIteration(context.Background())

Expand Down
6 changes: 0 additions & 6 deletions internal/nss/group/integrationtests.go
View file

This file was deleted.

6 changes: 0 additions & 6 deletions internal/nss/group/package_test.go
View file

This file was deleted.

6 changes: 0 additions & 6 deletions internal/nss/passwd/integrationtests.go
View file

This file was deleted.

6 changes: 0 additions & 6 deletions internal/nss/passwd/package_test.go
View file

This file was deleted.

20 changes: 6 additions & 14 deletions internal/nss/passwd/passwd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,8 @@ type Passwd struct {
shell string /* shell program */
}

var testopts = []cache.Option{}

// setCacheOption set opts everytime we open a cache.
// This is not compatible with parallel testing as it needs to change a global state.
func setCacheOption(opts ...cache.Option) {
testopts = opts
}

// NewByName returns a passwd entry from a name.
func NewByName(ctx context.Context, name string) (p Passwd, err error) {
func NewByName(ctx context.Context, name string, cacheOpts ...cache.Option) (p Passwd, err error) {
defer func() {
if err != nil {
err = fmt.Errorf("failed to get passwd entry from name %q: %w", name, err)
Expand All @@ -39,7 +31,7 @@ func NewByName(ctx context.Context, name string) (p Passwd, err error) {

logger.Debug(ctx, "Requesting a passwd entry matching name %q", name)

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return Passwd{}, nss.ConvertErr(err)
}
Expand All @@ -62,7 +54,7 @@ func NewByName(ctx context.Context, name string) (p Passwd, err error) {
}

// NewByUID returns a passwd entry from an UID.
func NewByUID(ctx context.Context, uid uint) (p Passwd, err error) {
func NewByUID(ctx context.Context, uid uint, cacheOpts ...cache.Option) (p Passwd, err error) {
defer func() {
if err != nil {
err = fmt.Errorf("failed to get passwd entry from UID %d: %w", uid, err)
Expand All @@ -71,7 +63,7 @@ func NewByUID(ctx context.Context, uid uint) (p Passwd, err error) {

logger.Debug(ctx, "Requesting a passwd entry matching UID %d", uid)

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return Passwd{}, nss.ConvertErr(err)
}
Expand All @@ -97,12 +89,12 @@ var passwdIterationCache *cache.Cache

// StartEntryIteration open a new cache for iteration.
// This needs to be called prior to calling NextEntry and be closed with EndEntryIteration.
func StartEntryIteration(ctx context.Context) error {
func StartEntryIteration(ctx context.Context, cacheOpts ...cache.Option) error {
if passwdIterationCache != nil {
return nss.ConvertErr(errors.New("passwd entry iteration already in progress. End it before starting a new one"))
}

c, err := cache.New(ctx, testopts...)
c, err := cache.New(ctx, cacheOpts...)
if err != nil {
return nss.ConvertErr(err)
}
Expand Down
28 changes: 15 additions & 13 deletions internal/nss/passwd/passwd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (

//nolint:dupl // TestNewByName and TestNewByGID have similar code that triggers dupl, despite being different.
func TestNewByName(t *testing.T) {
t.Parallel()

tests := map[string]struct {
name string
failingCache bool
Expand All @@ -29,6 +31,8 @@ func TestNewByName(t *testing.T) {
for name, tc := range tests {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()

cacheDir := t.TempDir()
testutils.PrepareDBsForTests(t, cacheDir, "users_in_db")

Expand All @@ -37,9 +41,8 @@ func TestNewByName(t *testing.T) {
if tc.failingCache {
opts = append(opts, cache.WithRootUID(4242))
}
passwd.SetCacheOption(opts...)

got, err := passwd.NewByName(context.Background(), tc.name)
got, err := passwd.NewByName(context.Background(), tc.name, opts...)
if tc.wantErrType != nil {
require.Error(t, err, "NewByName should have returned an error and hasn’t")
require.ErrorIs(t, err, tc.wantErrType, "NewByName has not returned expected error type")
Expand All @@ -55,6 +58,8 @@ func TestNewByName(t *testing.T) {

//nolint:dupl // TestNewByName and TestNewByUID have similar code that triggers dupl, despite being different.
func TestNewByUID(t *testing.T) {
t.Parallel()

tests := map[string]struct {
uid uint
failingCache bool
Expand All @@ -70,6 +75,8 @@ func TestNewByUID(t *testing.T) {
for name, tc := range tests {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()

cacheDir := t.TempDir()
testutils.PrepareDBsForTests(t, cacheDir, "users_in_db")

Expand All @@ -78,9 +85,8 @@ func TestNewByUID(t *testing.T) {
if tc.failingCache {
opts = append(opts, cache.WithRootUID(4242))
}
passwd.SetCacheOption(opts...)

got, err := passwd.NewByUID(context.Background(), tc.uid)
got, err := passwd.NewByUID(context.Background(), tc.uid, opts...)
if tc.wantErrType != nil {
require.Error(t, err, "NewByUID should have returned an error and hasn’t")
require.ErrorIs(t, err, tc.wantErrType, "NewByUID has not returned expected error type")
Expand Down Expand Up @@ -119,10 +125,9 @@ func TestNextEntry(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
passwd.SetCacheOption(opts...)

if !tc.noIterationInit {
err := passwd.StartEntryIteration(context.Background())
err := passwd.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "StartEntryIteration should succeed")
defer passwd.EndEntryIteration(context.Background())
}
Expand Down Expand Up @@ -175,21 +180,19 @@ func TestStartEndEntryIteration(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
passwd.SetCacheOption(opts...)

if tc.alreadyIterationInProgress {
err := passwd.StartEntryIteration(context.Background())
err := passwd.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "Setup: first startEntryIteration should have failed by hasn’t")
defer passwd.EndEntryIteration(context.Background())
}

if tc.cacheOpenError {
opts = append(opts, cache.WithRootUID(4242))
}
passwd.SetCacheOption(opts...)

if !tc.noStartIteration {
err := passwd.StartEntryIteration(context.Background())
err := passwd.StartEntryIteration(context.Background(), opts...)
if tc.wantStartIterationErr {
require.Error(t, err, "StartEntryIteration should have failed by hasn’t")
require.ErrorIs(t, err, nss.ErrUnavailableENoEnt, "Error should be of type Unavailable")
Expand All @@ -210,10 +213,9 @@ func TestRestartIterationWithoutEndingPreviousOne(t *testing.T) {

uid, gid := testutils.GetCurrentUIDGID(t)
opts := []cache.Option{cache.WithCacheDir(cacheDir), cache.WithRootUID(uid), cache.WithRootGID(gid), cache.WithShadowGID(gid)}
passwd.SetCacheOption(opts...)

// First iteration group
err := passwd.StartEntryIteration(context.Background())
err := passwd.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "StartEntryIteration should succeed")
defer passwd.EndEntryIteration(context.Background()) // in case of an error in the middle of the test. No-op otherwise

Expand All @@ -225,7 +227,7 @@ func TestRestartIterationWithoutEndingPreviousOne(t *testing.T) {
require.NoError(t, err, "EndEntryIteration while iterating should work")

// Second iteration group
err = passwd.StartEntryIteration(context.Background())
err = passwd.StartEntryIteration(context.Background(), opts...)
require.NoError(t, err, "restart a second entry iteration should succeed")
defer passwd.EndEntryIteration(context.Background())

Expand Down
6 changes: 0 additions & 6 deletions internal/nss/shadow/integrationtests.go
View file

This file was deleted.

6 changes: 0 additions & 6 deletions internal/nss/shadow/package_test.go
View file

This file was deleted.

0 comments on commit a60deb5

Please sign in to comment.