Fix async steps data race

.github/workflows/main.yml

@@ -30,6 +30,8 @@ jobs:
         run: cd ./pkg/allure && make test
       - name: provider-test
         run: cd ./pkg/framework && make test
+      - name: integration-test
+        run: make test
 
   examples:
     name: examples

Makefile

@@ -103,3 +103,7 @@ ifeq ($(wildcard $(GOLANGCI_BIN)),)
 		rm -rf $$tmp
 GOLANGCI_BIN:=$(LOCAL_BIN)/golangci-lint
 endif
+
+.PHONY: test
+test:
+	go test ./tests/... -race

pkg/allure/Makefile

@@ -59,4 +59,4 @@ endif
 
 .PHONY: test
 test:
-	go test ./... -cover
+	go test ./... -cover -race

pkg/allure/result.go

@@ -92,6 +92,54 @@ func (result *Result) SetStatusTrace(trace string) {
 	result.StatusDetails.Trace = trace
 }
 
+// AddAttachments appends test-level attachments.
+func (result *Result) AddAttachments(attachments ...*Attachment) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.Attachments = append(result.Attachments, attachments...)
+}
+
+// AddParameters appends test-level parameters.
+func (result *Result) AddParameters(params ...*Parameter) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.Parameters = append(result.Parameters, params...)
+}
+
+// AddLinks appends test-level links.
+func (result *Result) AddLinks(links ...*Link) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.Links = append(result.Links, links...)
+}
+
+// AddSteps appends test-level steps.
+func (result *Result) AddSteps(steps ...*Step) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.Steps = append(result.Steps, steps...)
+}
+
+// AddExpectedSteps appends expected steps.
+func (result *Result) AddExpectedSteps(steps ...*Step) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.ExpectedSteps = append(result.ExpectedSteps, steps...)
+}
+
+// SetExpectedResult sets expected result value.
+func (result *Result) SetExpectedResult(expected string) {
+	result.m.Lock()
+	defer result.m.Unlock()
+
+	result.ExpectedResult = expected
+}
+
 func (result *Result) GetStatusTrace() string {
 	return result.StatusDetails.Trace
 }

pkg/framework/Makefile

@@ -59,4 +59,4 @@ endif
 
 .PHONY: test
 test:
-	go test ./... -cover
+	go test ./... -cover -race

pkg/framework/core/allure_manager/ctx/test_ctx.go

@@ -17,7 +17,7 @@ func NewTestCtx(result *allure.Result) provider.ExecutionContext {
 }
 
 func (ctx *testCtx) AddStep(newStep *allure.Step) {
-	ctx.result.Steps = append(ctx.result.Steps, newStep)
+	ctx.result.AddSteps(newStep)
 }
 
 func (ctx *testCtx) GetName() string {
@@ -29,5 +29,5 @@ func (ctx *testCtx) GetTestResult() *allure.Result {
 }
 
 func (ctx *testCtx) AddAttachments(attachments ...*allure.Attachment) {
-	ctx.result.Attachments = append(ctx.result.Attachments, attachments...)
+	ctx.result.AddAttachments(attachments...)
 }

pkg/framework/core/allure_manager/manager/labels.go

@@ -10,30 +10,16 @@ Labels
 
 // Label provides possibility to add any Label to test result
 func (a *allureManager) Label(label *allure.Label) {
-	a.withResult(func(r *allure.Result) {
-		r.Labels = append(r.Labels, label)
-	})
+	a.withResult(func(r *allure.Result) { r.AddLabel(label) })
 }
 
 // Labels provides possibility to add few Labels to test result
 func (a *allureManager) Labels(labels ...*allure.Label) {
-	a.withResult(func(r *allure.Result) {
-		r.Labels = append(r.Labels, labels...)
-	})
+	a.withResult(func(r *allure.Result) { r.AddLabel(labels...) })
 }
 
 func (a *allureManager) ReplaceLabel(label *allure.Label) {
-	a.withResult(func(r *allure.Result) {
-		for _, l := range r.Labels {
-			if l.Name == label.Name {
-				l.Value = label.Value
-
-				return
-			}
-		}
-
-		a.Label(label)
-	})
+	a.withResult(func(r *allure.Result) { r.ReplaceLabel(label) })
 }
 
 // Epic adds Epic label to test result

pkg/framework/core/allure_manager/manager/links.go

@@ -27,13 +27,13 @@ func (a *allureManager) TmsLinks(testCase ...string) {
 // Link adds Link to struct.AllureResult
 func (a *allureManager) Link(link *allure.Link) {
 	a.withResult(func(r *allure.Result) {
-		r.Links = append(r.Links, link)
+		r.AddLinks(link)
 	})
 }
 
 // Links adds multiple Link to struct.AllureResult
 func (a *allureManager) Links(links []*allure.Link) {
 	a.withResult(func(r *allure.Result) {
-		r.Links = append(r.Links, links...)
+		r.AddLinks(links...)
 	})
 }

pkg/framework/core/allure_manager/manager/parameter.go

@@ -5,13 +5,13 @@ import "github.com/ozontech/allure-go/pkg/allure"
 // WithParameters adds parameters to report in case of current execution context
 func (a *allureManager) WithParameters(params ...*allure.Parameter) {
 	a.withResult(func(r *allure.Result) {
-		r.Parameters = append(r.Parameters, params...)
+		r.AddParameters(params...)
 	})
 }
 
 // WithNewParameters adds parameters to report in case of current execution context
 func (a *allureManager) WithNewParameters(kv ...interface{}) {
 	a.withResult(func(r *allure.Result) {
-		r.Parameters = append(r.Parameters, allure.NewParameters(kv...)...)
+		r.AddParameters(allure.NewParameters(kv...)...)
 	})
 }

pkg/framework/core/common/step_context_test.go

@@ -3,7 +3,6 @@ package common
 import (
 	"sync"
 	"testing"
-	"time"
 
 	"github.com/ozontech/allure-go/pkg/allure"
 	"github.com/ozontech/allure-go/pkg/framework/asserts_wrapper/helper"
@@ -419,20 +418,17 @@ func TestStepCtx_WithNewStep(t *testing.T) {
 }
 
 func TestStepCtx_WithNewAsyncStep(t *testing.T) {
-	wg := sync.WaitGroup{}
 	flag := false
-	wg.Add(1)
 	stepF := func(ctx provider.StepCtx) {
 		flag = true
-		defer wg.Done()
 	}
 
 	mockT := new(providerTMockStep)
 	step := allure.NewSimpleStep("testStep")
 
 	ctx := stepCtx{t: mockT, currentStep: step}
 	ctx.WithNewAsyncStep("new step", stepF, allure.NewParameter("p1", "v1"))
-	wg.Wait()
+	ctx.WG().Wait()
 	require.True(t, flag)
 	require.NotNil(t, ctx.currentStep.Steps)
 	require.NotEmpty(t, ctx.currentStep.Steps)
@@ -485,9 +481,7 @@ func TestStepCtx_WithNewAsyncStep_panic(t *testing.T) {
 
 	ctx := stepCtx{t: mockT, p: &providerMockStep{executionContext: newExecutionCtxMock(constants.TestContextName)}, currentStep: step}
 	ctx.WithNewAsyncStep("new step", stepF, allure.NewParameter("p1", "v1"))
-
-	// wg doesn't help cause panic
-	time.Sleep(100 * time.Millisecond)
+	ctx.WG().Wait()
 
 	require.True(t, flag)
 	require.True(t, mockT.errorF)

pkg/framework/core/common/steps_test.go

@@ -3,7 +3,6 @@ package common
 import (
 	"fmt"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -266,7 +265,7 @@ func TestCommon_WithNewAsyncStep(t *testing.T) {
 	comm := Common{TestingT: mockT, Provider: p}
 	params := allure.NewParameters("p1", "v1", "p2", "v2")
 	comm.WithNewAsyncStep("step", func(ctx provider.StepCtx) {}, params...)
-	time.Sleep(100 * time.Millisecond)
+	comm.WG().Wait()
 	require.NotEmpty(t, p.steps)
 	require.Len(t, p.steps, 1)
 	require.Equal(t, "step", p.steps[0].Name)
@@ -284,7 +283,7 @@ func TestCommon_WithNewAsyncStep_panic(t *testing.T) {
 	comm := Common{TestingT: mockT, Provider: p}
 	params := allure.NewParameters("p1", "v1", "p2", "v2")
 	comm.WithNewAsyncStep("step", func(ctx provider.StepCtx) { panic("whoops") }, params...)
-	time.Sleep(100 * time.Millisecond)
+	comm.WG().Wait()
 	require.NotEmpty(t, p.steps)
 	require.Len(t, p.steps, 1)
 	require.Equal(t, "step", p.steps[0].Name)

tests/async_step_test.go

@@ -0,0 +1,124 @@
+package tests
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/ozontech/allure-go/pkg/allure"
+	"github.com/ozontech/allure-go/pkg/framework/provider"
+	"github.com/ozontech/allure-go/pkg/framework/runner"
+)
+
+func TestAsyncStep_race(t *testing.T) {
+	t.Run("steps race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step race")
+		r.NewTest("async step race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				paramVal := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_%d", paramVal), func(sCtx provider.StepCtx) {
+					sCtx.WithNewParameters("param", paramVal)
+				})
+			}
+
+			// Wait for all async steps to be flushed by internal wait groups.
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+	t.Run("nested steps race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step nested stepctx race")
+		r.NewTest("async step nested stepctx race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				idx := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_add_step_%d", idx), func(ctx provider.StepCtx) {
+					ctx.WithNewStep(fmt.Sprintf("inner_%d", idx), func(inner provider.StepCtx) {
+						inner.WithNewParameters("idx", idx)
+					})
+				})
+			}
+
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+	t.Run("attachments race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step attachment race")
+		r.NewTest("async step attachment race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				idx := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_attach_%d", idx), func(ctx provider.StepCtx) {
+					t.WithNewAttachment(
+						fmt.Sprintf("attach_%d", idx),
+						"text/plain",
+						[]byte(fmt.Sprintf("payload-%d", idx)),
+					)
+					ctx.WithNewAttachment(
+						fmt.Sprintf("inner_attach_%d", idx),
+						"text/plain",
+						[]byte(fmt.Sprintf("inner-payload-%d", idx)),
+					)
+				})
+			}
+
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+	t.Run("parameters race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step parameter race")
+		r.NewTest("async step parameter race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				idx := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_param_%d", idx), func(ctx provider.StepCtx) {
+					t.WithNewParameters("idx", idx)
+					ctx.WithNewParameters("inner_idx", idx)
+				})
+			}
+
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+	t.Run("labels race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step label race")
+		r.NewTest("async step label race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				idx := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_label_%d", idx), func(ctx provider.StepCtx) {
+					t.Tag(fmt.Sprintf("tag_%d", idx))
+					t.ReplaceLabel(allure.NewLabel(allure.Feature, fmt.Sprintf("feature_%d", idx)))
+					ctx.WithNewParameters("label_idx", idx)
+				})
+			}
+
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+	t.Run("links race via async steps", func(t *testing.T) {
+		r := runner.NewRunner(t, "suite: async step link race")
+		r.NewTest("async step link race", func(t provider.T) {
+			for i := 0; i < 64; i++ {
+				idx := i
+				t.WithNewAsyncStep(fmt.Sprintf("step_link_%d", idx), func(ctx provider.StepCtx) {
+					t.Link(allure.NewLink(
+						fmt.Sprintf("link_%d", idx),
+						allure.LINK,
+						fmt.Sprintf("https://localhost/%d", idx),
+					))
+					ctx.WithNewParameters("link_idx", idx)
+				})
+			}
+
+			time.Sleep(300 * time.Millisecond)
+		})
+		r.RunTests()
+	})
+
+}