Skip to content

Commit 13d2212

Browse files
author
Michal Tichák
committed
fixup! [core] INVARIANT status introduced
1 parent 87151b8 commit 13d2212

File tree

2 files changed

+5
-1
lines changed

2 files changed

+5
-1
lines changed

core/workflow/safestatus.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ func aggregateStatus(roles []Role) (status task.Status) {
9595
return
9696
}
9797

98+
// TODO: this function is prime candidate for refactoring. The reason being that it mostly ignores status argument
99+
// for merging, moreover it also does not use status of role from argument. Both of these behaivour are counter-intuitive.
98100
func (t *SafeStatus) merge(s task.Status, r Role) {
99101
t.mu.Lock()
100102
defer t.mu.Unlock()

core/workflow/safestatus_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ var _ = Describe("safe status merge", func() {
6262
})
6363
})
6464

65-
When("Aggregate role is merged with any status (except UNDEFINED) status", func() {
65+
When("Empty aggregate role is merged with any status (except UNDEFINED) status", func() {
6666
It("becomes UNDEFINED", func() {
6767
ar := &aggregatorRole{roleBase: roleBase{status: SafeStatus{status: task.INACTIVE}}}
6868
ar.status.merge(task.ACTIVE, ar)
@@ -95,6 +95,8 @@ var _ = Describe("safe status merge", func() {
9595
},
9696
},
9797
}
98+
// SafeStatus.Merge basically ignores status argument in a lot of cases, so it does not matter what we use here
99+
// and in following tests
98100
ar.status.merge(task.INACTIVE, ar)
99101
Expect(ar.GetStatus()).To(Equal(task.Status(task.ACTIVE)))
100102
})

0 commit comments

Comments
 (0)