Skip to content

Commit

Permalink
fix: prefer asserts and prefix errors with pkg name
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris Stockton committed Feb 3, 2025
1 parent 55d046c commit e74faa2
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 48 deletions.
2 changes: 1 addition & 1 deletion hack/coverage.sh
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FAIL=false

for PKG in "crypto"
for PKG in "crypto" "reloader"
do
UNCOVERED_FUNCS=$(go tool cover -func=coverage.out | grep "^github.com/supabase/auth/internal/$PKG/" | grep -v '100.0%$')
UNCOVERED_FUNCS_COUNT=$(echo "$UNCOVERED_FUNCS" | wc -l)
Expand Down
15 changes: 8 additions & 7 deletions internal/reloader/reloader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (rl *Reloader) reloadCheckAt(at, lastUpdate time.Time) bool {
func (rl *Reloader) Watch(ctx context.Context, fn ConfigFunc) error {
wr, err := rl.watchFn()
if err != nil {
logrus.WithError(err).Error(err)
logrus.WithError(err).Error("reloader: error creating fsnotify Watcher")
return err
}
defer wr.Close()
Expand All @@ -75,7 +75,7 @@ func (rl *Reloader) Watch(ctx context.Context, fn ConfigFunc) error {

// Ignore errors, if watch dir doesn't exist we can add it later.
if err := wr.Add(rl.watchDir); err != nil {
logrus.WithError(err).Error(err)
logrus.WithError(err).Error("reloader: error watching config directory")
}

var lastUpdate time.Time
Expand Down Expand Up @@ -103,7 +103,7 @@ func (rl *Reloader) Watch(ctx context.Context, fn ConfigFunc) error {

cfg, err := rl.reload()
if err != nil {
logrus.WithError(err).Error(err)
logrus.WithError(err).Error("reloader: error loading config")
continue
}

Expand All @@ -112,8 +112,8 @@ func (rl *Reloader) Watch(ctx context.Context, fn ConfigFunc) error {

case evt, ok := <-wr.Events():
if !ok {
err := errors.New("fsnotify event channel was closed")
logrus.WithError(err).Error("fsnotify event channel was closed")
err := errors.New("reloader: fsnotify event channel was closed")
logrus.WithError(err).Error(err)
return err
}

Expand All @@ -131,11 +131,12 @@ func (rl *Reloader) Watch(ctx context.Context, fn ConfigFunc) error {
}
case err, ok := <-wr.Errors():
if !ok {
err := errors.New("fsnotify error channel was closed")
err := errors.New("reloader: fsnotify error channel was closed")
logrus.WithError(err).Error(err)
return err
}
logrus.WithError(err).Error("fsnotify has reported an error")
logrus.WithError(err).Error(
"reloader: fsnotify has reported an error")
}
}
}
Expand Down
70 changes: 30 additions & 40 deletions internal/reloader/reloader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestWatch(t *testing.T) {

err := rl.Watch(ctx, rr.configFn)
if exp, got := sentinelErr, err; exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.Equal(t, exp, got)
}
}

Expand All @@ -45,7 +45,7 @@ func TestWatch(t *testing.T) {
rl := NewReloader(path.Join(dir, "__not_found__"))
err := rl.Watch(doneCtx, rr.configFn)
if exp, got := context.Canceled, err; exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.Equal(t, exp, got)
}
}

Expand All @@ -60,11 +60,11 @@ func TestWatch(t *testing.T) {
rl.watchFn = func() (watcher, error) { return wr, nil }

err := rl.Watch(ctx, rr.configFn)
if err == nil {
t.Fatal("exp non-nil err")
}
if exp, got := "fsnotify error channel was closed", err.Error(); exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.NotNil(t, err)

msg := "reloader: fsnotify error channel was closed"
if exp, got := msg, err.Error(); exp != got {
assert.Equal(t, exp, got)
}
}

Expand All @@ -80,10 +80,12 @@ func TestWatch(t *testing.T) {

err := rl.Watch(ctx, rr.configFn)
if err == nil {
t.Fatal("exp non-nil err")
assert.NotNil(t, err)
}
if exp, got := "fsnotify event channel was closed", err.Error(); exp != got {
t.Fatalf("exp err %v; got %v", exp, got)

msg := "reloader: fsnotify event channel was closed"
if exp, got := msg, err.Error(); exp != got {
assert.Equal(t, exp, got)
}
}

Expand Down Expand Up @@ -125,7 +127,7 @@ func TestWatch(t *testing.T) {

err := eg.Wait()
if exp, got := context.Canceled, err; exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.Equal(t, exp, got)
}
}

Expand Down Expand Up @@ -153,7 +155,7 @@ func TestWatch(t *testing.T) {
{
select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case v := <-wr.addCh:
assert.Equal(t, v, dir)
}
Expand All @@ -165,7 +167,7 @@ func TestWatch(t *testing.T) {
}
select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case cfg := <-rr.configCh:
assert.NotNil(t, cfg)
assert.Equal(t, cfg.External.Apple.Enabled, false)
Expand All @@ -185,7 +187,7 @@ func TestWatch(t *testing.T) {
}
select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case cfg := <-rr.configCh:
assert.NotNil(t, cfg)
assert.Equal(t, cfg.External.Apple.Enabled, true)
Expand Down Expand Up @@ -216,10 +218,10 @@ func TestWatch(t *testing.T) {

select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case p := <-rr.reloadCh:
if exp, got := dir, p; exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.Equal(t, exp, got)
}
}
}
Expand All @@ -234,7 +236,7 @@ func TestWatch(t *testing.T) {
}
select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case cfg := <-rr.configCh:
assert.NotNil(t, cfg)
assert.Equal(t, cfg.SMTP.Port, 2222)
Expand All @@ -256,7 +258,7 @@ func TestWatch(t *testing.T) {
}
select {
case <-egCtx.Done():
t.Fatalf("exp nil err; got %v", egCtx.Err())
assert.Nil(t, egCtx.Err())
case cfg := <-rr.configCh:
assert.NotNil(t, cfg)
assert.Equal(t, cfg.SMTP.Port, 2222)
Expand All @@ -268,7 +270,7 @@ func TestWatch(t *testing.T) {

err := eg.Wait()
if exp, got := context.Canceled, err; exp != got {
t.Fatalf("exp err %v; got %v", exp, got)
assert.Equal(t, exp, got)
}
}
}
Expand All @@ -283,9 +285,7 @@ func TestReloadConfig(t *testing.T) {
helpCopyEnvFile(t, dir, "01_example.env", "testdata/50_example.env")
{
cfg, err := rl.reload()
if err != nil {
t.Fatal(err)
}
assert.Nil(t, err)
assert.NotNil(t, cfg)
assert.Equal(t, cfg.External.Apple.Enabled, false)
}
Expand All @@ -295,9 +295,7 @@ func TestReloadConfig(t *testing.T) {
})
{
cfg, err := rl.reload()
if err != nil {
t.Fatal(err)
}
assert.Nil(t, err)
assert.NotNil(t, cfg)
assert.Equal(t, cfg.External.Apple.Enabled, true)
}
Expand All @@ -307,9 +305,7 @@ func TestReloadConfig(t *testing.T) {
})
{
cfg, err := rl.reload()
if err != nil {
t.Fatal(err)
}
assert.Nil(t, err)
assert.NotNil(t, cfg)
assert.Equal(t, cfg.External.Apple.Enabled, true)
}
Expand All @@ -321,9 +317,7 @@ func TestReloadConfig(t *testing.T) {
})
{
cfg, err := rl.reload()
if err == nil {
t.Fatal("exp non-nil error")
}
assert.NotNil(t, err)
assert.Nil(t, cfg)
}

Expand All @@ -332,9 +326,7 @@ func TestReloadConfig(t *testing.T) {
cleanup()

cfg, err := rl.reload()
if err == nil {
t.Fatal("exp non-nil error")
}
assert.NotNil(t, err)
assert.Nil(t, cfg)
}
}
Expand Down Expand Up @@ -422,21 +414,21 @@ func helpTestDir(t testing.TB) (dir string, cleanup func()) {
dir = filepath.Join("testdata", t.Name())
err := os.MkdirAll(dir, 0750)
if err != nil && !os.IsExist(err) {
t.Fatal(err)
assert.Nil(t, err)
}
return dir, func() { os.RemoveAll(dir) }
}

func helpCopyEnvFile(t testing.TB, dir, name, src string) string {
data, err := os.ReadFile(src) // #nosec G304
if err != nil {
t.Fatal(err)
assert.Nil(t, err)
}

dst := filepath.Join(dir, name)
err = os.WriteFile(dst, data, 0600)
if err != nil {
t.Fatal(err)
assert.Nil(t, err)
}
return dst
}
Expand All @@ -452,9 +444,7 @@ func helpWriteEnvFile(t testing.TB, dir, name string, values map[string]string)

dst := filepath.Join(dir, name)
err := os.WriteFile(dst, buf.Bytes(), 0600)
if err != nil {
t.Fatal(err)
}
assert.Nil(t, err)
return dst
}

Expand Down

0 comments on commit e74faa2

Please sign in to comment.