Does not throw err in the notification job (#11363)

Fixes #11280, no error return but just log.

Signed-off-by: wang yan <wangyan@vmware.com>

src/jobservice/job/impl/notification/webhook_job.go

@@ -52,7 +52,12 @@ func (wj *WebhookJob) Run(ctx job.Context, params job.Parameters) error {
 		return err
 	}
 
-	return wj.execute(ctx, params)
+	// does not throw err in the notification job
+	if err := wj.execute(ctx, params); err != nil {
+		wj.logger.Error(err)
+	}
+
+	return nil
 }
 
 // init webhook job

src/jobservice/job/impl/notification/webhook_job_test.go

@@ -1,7 +1,7 @@
 package notification
 
 import (
-	"github.com/goharbor/harbor/src/jobservice/job/impl"
+	mockjobservice "github.com/goharbor/harbor/src/testing/jobservice"
 	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"net/http"
@@ -35,6 +35,11 @@ func TestValidate(t *testing.T) {
 }
 
 func TestRun(t *testing.T) {
+	ctx := &mockjobservice.MockJobContext{}
+	logger := &mockjobservice.MockJobLogger{}
+
+	ctx.On("GetLogger").Return(logger)
+
 	rep := &WebhookJob{}
 
 	// test webhook request
@@ -57,7 +62,7 @@ func TestRun(t *testing.T) {
 		"auth_header":      "auth_test",
 	}
 	// test correct webhook response
-	assert.Nil(t, rep.Run(&impl.Context{}, params))
+	assert.Nil(t, rep.Run(ctx, params))
 
 	tsWrong := httptest.NewServer(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -71,5 +76,5 @@ func TestRun(t *testing.T) {
 		"auth_header":      "auth_test",
 	}
 	// test incorrect webhook response
-	assert.NotNil(t, rep.Run(&impl.Context{}, paramsWrong))
+	assert.Nil(t, rep.Run(ctx, paramsWrong))
 }

src/pkg/scan/job_test.go

@@ -15,19 +15,17 @@
 package scan
 
 import (
-	"context"
 	"encoding/json"
 	"testing"
 	"time"
 
 	"github.com/goharbor/harbor/src/jobservice/job"
-	"github.com/goharbor/harbor/src/jobservice/logger"
 	"github.com/goharbor/harbor/src/pkg/scan/dao/scanner"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 	"github.com/goharbor/harbor/src/pkg/scan/vuln"
+	mockjobservice "github.com/goharbor/harbor/src/testing/jobservice"
 	mocktesting "github.com/goharbor/harbor/src/testing/mock"
 	v1testing "github.com/goharbor/harbor/src/testing/pkg/scan/rest/v1"
-	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
@@ -61,8 +59,8 @@ func (suite *JobTestSuite) TeraDownSuite() {
 
 // TestJob tests the scan job
 func (suite *JobTestSuite) TestJob() {
-	ctx := &MockJobContext{}
+	ctx := &mockjobservice.MockJobContext{}
-	lg := &MockJobLogger{}
+	lg := &mockjobservice.MockJobLogger{}
 
 	ctx.On("GetLogger").Return(lg)
 
@@ -146,114 +144,3 @@ func (suite *JobTestSuite) TestJob() {
 	err = j.Run(ctx, jp)
 	require.NoError(suite.T(), err)
 }
-
-// MockJobContext mocks job context interface.
-// TODO: Maybe moved to a separate `mock` pkg for sharing in future.
-type MockJobContext struct {
-	mock.Mock
-}
-
-// Build ...
-func (mjc *MockJobContext) Build(tracker job.Tracker) (job.Context, error) {
-	args := mjc.Called(tracker)
-	c := args.Get(0)
-	if c != nil {
-		return c.(job.Context), nil
-	}
-
-	return nil, args.Error(1)
-}
-
-// Get ...
-func (mjc *MockJobContext) Get(prop string) (interface{}, bool) {
-	args := mjc.Called(prop)
-	return args.Get(0), args.Bool(1)
-}
-
-// SystemContext ...
-func (mjc *MockJobContext) SystemContext() context.Context {
-	return context.TODO()
-}
-
-// Checkin ...
-func (mjc *MockJobContext) Checkin(status string) error {
-	args := mjc.Called(status)
-	return args.Error(0)
-}
-
-// OPCommand ...
-func (mjc *MockJobContext) OPCommand() (job.OPCommand, bool) {
-	args := mjc.Called()
-	return (job.OPCommand)(args.String(0)), args.Bool(1)
-}
-
-// GetLogger ...
-func (mjc *MockJobContext) GetLogger() logger.Interface {
-	return &MockJobLogger{}
-}
-
-// Tracker ...
-func (mjc *MockJobContext) Tracker() job.Tracker {
-	args := mjc.Called()
-	if t := args.Get(0); t != nil {
-		return t.(job.Tracker)
-	}
-
-	return nil
-}
-
-// MockJobLogger mocks the job logger interface.
-// TODO: Maybe moved to a separate `mock` pkg for sharing in future.
-type MockJobLogger struct {
-	mock.Mock
-}
-
-// Debug ...
-func (mjl *MockJobLogger) Debug(v ...interface{}) {
-	logger.Debug(v...)
-}
-
-// Debugf ...
-func (mjl *MockJobLogger) Debugf(format string, v ...interface{}) {
-	logger.Debugf(format, v...)
-}
-
-// Info ...
-func (mjl *MockJobLogger) Info(v ...interface{}) {
-	logger.Info(v...)
-}
-
-// Infof ...
-func (mjl *MockJobLogger) Infof(format string, v ...interface{}) {
-	logger.Infof(format, v...)
-}
-
-// Warning ...
-func (mjl *MockJobLogger) Warning(v ...interface{}) {
-	logger.Warning(v...)
-}
-
-// Warningf ...
-func (mjl *MockJobLogger) Warningf(format string, v ...interface{}) {
-	logger.Warningf(format, v...)
-}
-
-// Error ...
-func (mjl *MockJobLogger) Error(v ...interface{}) {
-	logger.Error(v...)
-}
-
-// Errorf ...
-func (mjl *MockJobLogger) Errorf(format string, v ...interface{}) {
-	logger.Errorf(format, v...)
-}
-
-// Fatal ...
-func (mjl *MockJobLogger) Fatal(v ...interface{}) {
-	logger.Fatal(v...)
-}
-
-// Fatalf ...
-func (mjl *MockJobLogger) Fatalf(format string, v ...interface{}) {
-	logger.Fatalf(format, v...)
-}

src/testing/jobservice/context.go

@@ -0,0 +1,119 @@
+package jobservice
+
+import (
+	"context"
+	"github.com/goharbor/harbor/src/jobservice/job"
+	"github.com/goharbor/harbor/src/jobservice/logger"
+	"github.com/stretchr/testify/mock"
+)
+
+// MockJobContext mocks job context interface.
+// TODO: Maybe moved to a separate `mock` pkg for sharing in future.
+type MockJobContext struct {
+	mock.Mock
+}
+
+// Build ...
+func (mjc *MockJobContext) Build(tracker job.Tracker) (job.Context, error) {
+	args := mjc.Called(tracker)
+	c := args.Get(0)
+	if c != nil {
+		return c.(job.Context), nil
+	}
+
+	return nil, args.Error(1)
+}
+
+// Get ...
+func (mjc *MockJobContext) Get(prop string) (interface{}, bool) {
+	args := mjc.Called(prop)
+	return args.Get(0), args.Bool(1)
+}
+
+// SystemContext ...
+func (mjc *MockJobContext) SystemContext() context.Context {
+	return context.TODO()
+}
+
+// Checkin ...
+func (mjc *MockJobContext) Checkin(status string) error {
+	args := mjc.Called(status)
+	return args.Error(0)
+}
+
+// OPCommand ...
+func (mjc *MockJobContext) OPCommand() (job.OPCommand, bool) {
+	args := mjc.Called()
+	return (job.OPCommand)(args.String(0)), args.Bool(1)
+}
+
+// GetLogger ...
+func (mjc *MockJobContext) GetLogger() logger.Interface {
+	return &MockJobLogger{}
+}
+
+// Tracker ...
+func (mjc *MockJobContext) Tracker() job.Tracker {
+	args := mjc.Called()
+	if t := args.Get(0); t != nil {
+		return t.(job.Tracker)
+	}
+
+	return nil
+}
+
+// MockJobLogger mocks the job logger interface.
+// TODO: Maybe moved to a separate `mock` pkg for sharing in future.
+type MockJobLogger struct {
+	mock.Mock
+}
+
+// Debug ...
+func (mjl *MockJobLogger) Debug(v ...interface{}) {
+	logger.Debug(v...)
+}
+
+// Debugf ...
+func (mjl *MockJobLogger) Debugf(format string, v ...interface{}) {
+	logger.Debugf(format, v...)
+}
+
+// Info ...
+func (mjl *MockJobLogger) Info(v ...interface{}) {
+	logger.Info(v...)
+}
+
+// Infof ...
+func (mjl *MockJobLogger) Infof(format string, v ...interface{}) {
+	logger.Infof(format, v...)
+}
+
+// Warning ...
+func (mjl *MockJobLogger) Warning(v ...interface{}) {
+	logger.Warning(v...)
+}
+
+// Warningf ...
+func (mjl *MockJobLogger) Warningf(format string, v ...interface{}) {
+	logger.Warningf(format, v...)
+}
+
+// Error ...
+func (mjl *MockJobLogger) Error(v ...interface{}) {
+	logger.Error(v...)
+}
+
+// Errorf ...
+func (mjl *MockJobLogger) Errorf(format string, v ...interface{}) {
+	logger.Errorf(format, v...)
+}
+
+// Fatal ...
+func (mjl *MockJobLogger) Fatal(v ...interface{}) {
+	logger.Fatal(v...)
+}
+
+// Fatalf ...
+func (mjl *MockJobLogger) Fatalf(format string, v ...interface{}) {
+	logger.Fatalf(format, v...)
+}