Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run integration tests again #7304

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Run integration tests again #7304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,13 @@ prepare-coverage-test: $(GOTESTSUM) $(TEST_OUTPUT_ROOT)

unit-test-coverage: prepare-coverage-test
@printf $(COLOR) "Run unit tests with coverage..."
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries $(FAILED_TEST_RETRIES) --junitfile $(NEW_REPORT) --packages $(UNIT_TEST_DIRS) -- \
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(UNIT_TEST_DIRS)" -- \
$(COMPILED_TEST_ARGS) \
-coverprofile=$(NEW_COVER_PROFILE)

integration-test-coverage: prepare-coverage-test
@printf $(COLOR) "Run integration tests with coverage..."
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries $(FAILED_TEST_RETRIES) --junitfile $(NEW_REPORT) --packages $(INTEGRATION_TEST_DIRS) -- \
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(INTEGRATION_TEST_DIRS)" -- \
$(COMPILED_TEST_ARGS) \
-coverprofile=$(NEW_COVER_PROFILE) $(INTEGRATION_TEST_COVERPKG)

Expand All @@ -429,21 +429,21 @@ pre-build-functional-test-coverage: prepare-coverage-test

functional-test-coverage: prepare-coverage-test
@printf $(COLOR) "Run functional tests with coverage with $(PERSISTENCE_DRIVER) driver..."
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries $(FAILED_TEST_RETRIES) --junitfile $(NEW_REPORT) --packages $(FUNCTIONAL_TEST_ROOT) -- \
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_ROOT)" -- \
$(COMPILED_TEST_ARGS) \
-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)

functional-test-xdc-coverage: prepare-coverage-test
@printf $(COLOR) "Run functional test for cross DC with coverage with $(PERSISTENCE_DRIVER) driver..."
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries $(FAILED_TEST_RETRIES) --junitfile $(NEW_REPORT) --packages $(FUNCTIONAL_TEST_XDC_ROOT) -- \
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_XDC_ROOT)" -- \
$(COMPILED_TEST_ARGS) \
-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)

functional-test-ndc-coverage: prepare-coverage-test
@printf $(COLOR) "Run functional test for NDC with coverage with $(PERSISTENCE_DRIVER) driver..."
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries $(FAILED_TEST_RETRIES) --junitfile $(NEW_REPORT) --packages $(FUNCTIONAL_TEST_NDC_ROOT) -- \
go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_NDC_ROOT)" -- \
$(COMPILED_TEST_ARGS) \
-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)
Expand Down
2 changes: 1 addition & 1 deletion common/persistence/tests/sqlite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ func TestSQLiteTransactionContextCancellation(t *testing.T) {
cancel()

err = tx.Commit()
assert.ErrorAs(t, err, &context.Canceled)
Copy link
Contributor Author

@stephanos stephanos Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This yields a data race when the assertion library is trying to wrap the error.

Example: https://github.com/temporalio/temporal/actions/runs/13249022301/job/36982452711?pr=7262#step:7:27

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this could possible work at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷

assert.ErrorIs(t, err, context.Canceled)

// Check if we still have a connection to the db.
_, err = db.LockTaskQueues(context.Background(), sqlplugin.TaskQueuesFilter{
Expand Down
17 changes: 2 additions & 15 deletions tools/tdbg/dlq_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type (
}
)

func (tc *dlqTestCase) Run(t *testing.T, firstAppRun chan struct{}) {
func (tc *dlqTestCase) Run(t *testing.T) {
faultyAdminClient := &fakeAdminClient{err: errors.New("did not expect client to be used")}
p := dlqTestParams{
dlqVersion: tc.version,
Expand Down Expand Up @@ -116,16 +116,8 @@ func (tc *dlqTestCase) Run(t *testing.T, firstAppRun chan struct{}) {
}
runArgs = appendArg(runArgs, tdbg.FlagOutputFilename, p.outputFileName)

// TODO: this is a hack to make sure that the first app.Run() call is finished before the second one starts because
// there is a race condition in the CLI app where all apps share the same help command pointer and try to initialize
// it at the same time. This workaround only protects the first call because it's ok if subsequent calls happen in
// parallel since the help command is already initialized.
_, isFirstRun := <-firstAppRun
t.Logf("Running %v", runArgs)
err := app.Run(runArgs)
if isFirstRun {
close(firstAppRun)
}
if len(p.expectedErrSubstrings) > 0 {
assert.Error(t, err, "Expected error to contain %v", p.expectedErrSubstrings)
for _, s := range p.expectedErrSubstrings {
Expand All @@ -144,10 +136,6 @@ func (tc *dlqTestCase) Run(t *testing.T, firstAppRun chan struct{}) {
}

func TestDLQCommand_V2(t *testing.T) {
t.Parallel()

firstAppRun := make(chan struct{}, 1)
firstAppRun <- struct{}{}
for _, tc := range []dlqTestCase{
{
name: "read no target cluster with faulty admin client",
Expand Down Expand Up @@ -309,9 +297,8 @@ func TestDLQCommand_V2(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No data race if you don't run it in parallel :smart:

tc.version = "v2"
tc.Run(t, firstAppRun)
tc.Run(t)
})
}
}
Expand Down
Loading