chia7712 commented on code in PR #912:
URL: https://github.com/apache/yunikorn-core/pull/912#discussion_r1670569036
##########
pkg/events/event_system_test.go:
##########
@@ -49,7 +49,11 @@ func TestSimpleStartAndStop(t *testing.T) {
// should be retrieved from the EventStore
func TestSingleEventStoredCorrectly(t *testing.T) {
Init()
- eventSystem := GetEventSystem().(*EventSystemImpl) //nolint:errcheck
+ eventSystem, ok := GetEventSystem().(*EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
we don't need this `return`, right?
##########
pkg/log/logger_test.go:
##########
@@ -243,7 +243,9 @@ func TestParentLogger(t *testing.T) {
func resetTestLogger() {
// flush log
- logger.Sync() //nolint:errcheck
+ if err := logger.Sync(); err != nil {
+ t.Fatalf("Error syncing logger: %s", err.Error())
Review Comment:
please use `assert.NilError`
##########
pkg/scheduler/objects/application_test.go:
##########
@@ -2198,7 +2214,11 @@ func TestPlaceholderLargerEvent(t *testing.T) {
app := newApplication(appID1, "default", "root.default")
// Create event system after new application to avoid new application
event.
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/events/event_system_test.go:
##########
@@ -114,7 +122,11 @@ func TestConfigUpdate(t *testing.T) {
defer configs.SetConfigMap(map[string]string{})
Init()
- eventSystem := GetEventSystem().(*EventSystemImpl) //nolint:errcheck
+ eventSystem, ok := GetEventSystem().(*EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/events/event_system_test.go:
##########
@@ -82,7 +86,11 @@ func TestSingleEventStoredCorrectly(t *testing.T) {
func TestGetEvents(t *testing.T) {
Init()
- eventSystem := GetEventSystem().(*EventSystemImpl) //nolint:errcheck
+ eventSystem, ok := GetEventSystem().(*EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/objects/application_test.go:
##########
@@ -380,7 +384,11 @@ func TestAddAllocAsk(t *testing.T) {
app := newApplication(appID1, "default", "root.unknown")
// Create event system after new application to avoid new application
event.
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/partition_test.go:
##########
@@ -3817,7 +3817,11 @@ func TestTryAllocateMaxRunning(t *testing.T) {
func TestNewQueueEvents(t *testing.T) {
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/objects/queue_test.go:
##########
@@ -2455,7 +2455,11 @@ func TestQueue_setAllocatingAccepted(t *testing.T) {
func TestQueueEvents(t *testing.T) {
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/objects/application_test.go:
##########
@@ -2079,7 +2091,11 @@ func TestAllocationEvents(t *testing.T) { //nolint:funlen
app := newApplication(appID1, "default", "root.default")
// Create event system after new application to avoid new app event.
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/objects/application_test.go:
##########
@@ -2013,7 +2021,11 @@ func TestAskEvents(t *testing.T) {
app := newApplication(appID1, "default", "root.default")
// Create event system after new application to avoid new app event.
events.Init()
- eventSystem := events.GetEventSystem().(*events.EventSystemImpl)
//nolint:errcheck
+ eventSystem, ok := events.GetEventSystem().(*events.EventSystemImpl)
+ if !ok {
+ t.Fatalf("Failed to cast GetEventSystem() to *EventSystemImpl")
+ return
Review Comment:
ditto
##########
pkg/scheduler/objects/application_state.go:
##########
@@ -140,7 +140,11 @@ func eventDesc() fsm.Events {
func callbacks() fsm.Callbacks {
return fsm.Callbacks{
"enter_state": func(_ context.Context, event *fsm.Event) {
- app := event.Args[0].(*Application) //nolint:errcheck
+ app, ok := event.Args[0].(*Application)
Review Comment:
This will increase complexity. Maybe we can keep origin `nolint`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
