From 8cb7d62e32fbc78e9bc6311f236372fdcad2c856 Mon Sep 17 00:00:00 2001 From: Nikita Turchaninov <2010628+slowaner@users.noreply.github.com> Date: Mon, 4 May 2026 12:06:15 +0300 Subject: [PATCH] Fix async steps data race --- .github/workflows/main.yml | 2 + Makefile | 4 + pkg/allure/Makefile | 2 +- pkg/allure/result.go | 48 +++++++ pkg/framework/Makefile | 2 +- .../core/allure_manager/ctx/test_ctx.go | 4 +- .../core/allure_manager/manager/labels.go | 20 +-- .../core/allure_manager/manager/links.go | 4 +- .../core/allure_manager/manager/parameter.go | 4 +- .../core/common/step_context_test.go | 10 +- pkg/framework/core/common/steps_test.go | 5 +- tests/async_step_test.go | 124 ++++++++++++++++++ 12 files changed, 193 insertions(+), 36 deletions(-) create mode 100644 tests/async_step_test.go diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7446f7c..c8d8ace 100644 --- a/.github/workflows/main.yml +++ b/.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 diff --git a/Makefile b/Makefile index d42d7da..19fd8c9 100644 --- a/Makefile +++ b/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 \ No newline at end of file diff --git a/pkg/allure/Makefile b/pkg/allure/Makefile index 0ff7d11..a2ab7cd 100644 --- a/pkg/allure/Makefile +++ b/pkg/allure/Makefile @@ -59,4 +59,4 @@ endif .PHONY: test test: - go test ./... -cover \ No newline at end of file + go test ./... -cover -race \ No newline at end of file diff --git a/pkg/allure/result.go b/pkg/allure/result.go index b73f949..bf7095e 100644 --- a/pkg/allure/result.go +++ b/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 } diff --git a/pkg/framework/Makefile b/pkg/framework/Makefile index 0ff7d11..a2ab7cd 100644 --- a/pkg/framework/Makefile +++ b/pkg/framework/Makefile @@ -59,4 +59,4 @@ endif .PHONY: test test: - go test ./... -cover \ No newline at end of file + go test ./... -cover -race \ No newline at end of file diff --git a/pkg/framework/core/allure_manager/ctx/test_ctx.go b/pkg/framework/core/allure_manager/ctx/test_ctx.go index ae05f28..57ea536 100644 --- a/pkg/framework/core/allure_manager/ctx/test_ctx.go +++ b/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...) } diff --git a/pkg/framework/core/allure_manager/manager/labels.go b/pkg/framework/core/allure_manager/manager/labels.go index 3c9e70b..8ca4f76 100644 --- a/pkg/framework/core/allure_manager/manager/labels.go +++ b/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 diff --git a/pkg/framework/core/allure_manager/manager/links.go b/pkg/framework/core/allure_manager/manager/links.go index 6edf093..0b04199 100644 --- a/pkg/framework/core/allure_manager/manager/links.go +++ b/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...) }) } diff --git a/pkg/framework/core/allure_manager/manager/parameter.go b/pkg/framework/core/allure_manager/manager/parameter.go index 4833e14..174ef40 100644 --- a/pkg/framework/core/allure_manager/manager/parameter.go +++ b/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...)...) }) } diff --git a/pkg/framework/core/common/step_context_test.go b/pkg/framework/core/common/step_context_test.go index 76ef72f..a311e39 100644 --- a/pkg/framework/core/common/step_context_test.go +++ b/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,12 +418,9 @@ 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) @@ -432,7 +428,7 @@ func TestStepCtx_WithNewAsyncStep(t *testing.T) { 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) diff --git a/pkg/framework/core/common/steps_test.go b/pkg/framework/core/common/steps_test.go index 618645b..077679c 100644 --- a/pkg/framework/core/common/steps_test.go +++ b/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) diff --git a/tests/async_step_test.go b/tests/async_step_test.go new file mode 100644 index 0000000..c01998c --- /dev/null +++ b/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() + }) + +}