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

Jump to bottom

Separate integration tests and unit tests #3760

Merged
merged 11 commits into from
Jan 5, 2023
Merged

Separate integration tests and unit tests #3760

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e638c74
Separate integration tests and unit tests
yux0 Dec 22, 2022
50de6dc
fix import
yux0 Dec 22, 2022
aab7c7e
update buildkite pipeline
yux0 Dec 22, 2022
f3b37a5
update coverage script
yux0 Dec 22, 2022
14e935a
Merge branch 'master' into test-target
yux0 Dec 22, 2022
dedcde7
Merge branch 'master' into test-target
yux0 Dec 31, 2022
5155dc5
optimize unit test run
yux0 Jan 4, 2023
9b1362c
remove foreach in test
yux0 Jan 4, 2023
416017f
Merge branch 'master' into test-target
yux0 Jan 4, 2023
7fda9eb
fix db tool test coverage
yux0 Jan 4, 2023
b956827
rename variable
yux0 Jan 5, 2023
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
28 changes: 25 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,14 @@ endef

TEST_TIMEOUT := 20m

# TODO: INTEG_TEST should be functional tests
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to address these TODOs first (in separate PR). Otherwise, we gonna have two different integration test sets and w/o looking at this PR it would be extremely hard to understand "why?".

Copy link
Member

Choose a reason for hiding this comment

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

Or you can do it right after. Just please don't postpone it.

INTEG_TEST_ROOT := ./host
Copy link
Contributor

Choose a reason for hiding this comment

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

How about renaming this ./host dir, too, to something like ./host_tests? It could have a clearer directory name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do in the following PR

INTEG_TEST_XDC_ROOT := ./host/xdc
INTEG_TEST_NDC_ROOT := ./host/ndc

INTEGRATION_TEST_ROOT := ./common/persistence/tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: DB_INTEGRATION_TEST_ROOT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also include ES tests and any future dependency integration tests. So I want to just use integration tests.

Copy link
Member

Choose a reason for hiding this comment

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

ES is also DB in this context. The path clearly points to persistence integration tests. If we have more integration tests in future, we will have another root and another var for it. So I would call this: PERSISTENCE_INTEGRATION_TEST_ROOT (yes, long, but clear).

DB_TOOL_INTEGRATION_TEST_ROOT := ./tools/tests

PROTO_ROOT := proto
PROTO_FILES = $(shell find ./$(PROTO_ROOT)/internal -name "*.proto")
PROTO_DIRS = $(sort $(dir $(PROTO_FILES)))
Expand All @@ -75,7 +79,7 @@ ALL_SCRIPTS := $(shell find . -name "*.sh")
# TODO (jeremy): Replace below with build tags and `go test ./...` for targets
TEST_DIRS := $(sort $(dir $(filter %_test.go,$(ALL_SRC))))
INTEG_TEST_DIRS := $(filter $(INTEG_TEST_ROOT)/ $(INTEG_TEST_NDC_ROOT)/,$(TEST_DIRS))
UNIT_TEST_DIRS := $(filter-out $(INTEG_TEST_ROOT)% $(INTEG_TEST_XDC_ROOT)% $(INTEG_TEST_NDC_ROOT)%,$(TEST_DIRS))
UNIT_TEST_DIRS := $(filter-out $(INTEG_TEST_ROOT)% $(INTEG_TEST_XDC_ROOT)% $(INTEG_TEST_NDC_ROOT)% $(INTEGRATION_TEST_ROOT)% $(DB_TOOL_INTEGRATION_TEST_ROOT)%,$(TEST_DIRS))

# go.opentelemetry.io/otel/sdk/metric@v0.31.0 - there are breaking changes in v0.32.0.
# github.com/urfave/cli/v2@v2.4.0 - needs to accept comma in values before unlocking https://github.com/urfave/cli/pull/1241.
Expand All @@ -87,6 +91,8 @@ PINNED_DEPENDENCIES := \
# Code coverage output files.
COVER_ROOT := ./.coverage
UNIT_COVER_PROFILE := $(COVER_ROOT)/unit_coverprofile.out
INTEGRATION_COVER_PROFILE := $(COVER_ROOT)/integration_coverprofile.out
DB_TOOL_COVER_PROFILE := $(COVER_ROOT)/db_tool_coverprofile.out
INTEG_COVER_PROFILE := $(COVER_ROOT)/integ_$(PERSISTENCE_DRIVER)_coverprofile.out
INTEG_XDC_COVER_PROFILE := $(COVER_ROOT)/integ_xdc_$(PERSISTENCE_DRIVER)_coverprofile.out
INTEG_NDC_COVER_PROFILE := $(COVER_ROOT)/integ_ndc_$(PERSISTENCE_DRIVER)_coverprofile.out
Expand Down Expand Up @@ -254,13 +260,20 @@ build-tests:
@printf $(COLOR) "Build tests..."
@go test -exec="true" -count=0 -tags=esintegration $(TEST_DIRS)

unit-test:
unit-test: clean-test-results
@printf $(COLOR) "Run unit tests..."
$(foreach UNIT_TEST_DIR,$(UNIT_TEST_DIRS),\
@go test $(UNIT_TEST_DIR) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) -race | tee -a test.log \
$(NEWLINE))
@! grep -q "^--- FAIL" test.log

db-integration-test: clean-test-results
@printf $(COLOR) "Run integration tests..."
@go test $(INTEGRATION_TEST_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) | tee -a test.log
@go test $(DB_TOOL_INTEGRATION_TEST_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) | tee -a test.log
@! grep -q "^--- FAIL" test.log

# TODO: rename it to functional-test
integration-test: clean-test-results
@printf $(COLOR) "Run integration tests..."
$(foreach INTEG_TEST_DIR,$(INTEG_TEST_DIRS),\
Expand All @@ -270,6 +283,7 @@ integration-test: clean-test-results
@go test $(INTEG_TEST_XDC_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) | tee -a test.log
@! grep -q "^--- FAIL" test.log

# TODO: rename it to functional-test
integration-with-fault-injection-test: clean-test-results
@printf $(COLOR) "Run integration tests with fault injection..."
$(foreach INTEG_TEST_DIR,$(INTEG_TEST_DIRS),\
Expand All @@ -279,7 +293,7 @@ integration-with-fault-injection-test: clean-test-results
@go test $(INTEG_TEST_XDC_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) -PersistenceFaultInjectionRate=0.005 | tee -a test.log
@! grep -q "^--- FAIL" test.log

test: unit-test integration-test integration-with-fault-injection-test
test: unit-test db-integration-test integration-test integration-with-fault-injection-test

##### Coverage #####
$(COVER_ROOT):
Expand All @@ -294,14 +308,22 @@ unit-test-coverage: $(COVER_ROOT)
grep -v -e "^mode: \w\+" $(COVER_ROOT)/$(UNIT_TEST_DIR)/coverprofile.out >> $(UNIT_COVER_PROFILE) || true \
$(NEWLINE))

db-integration-test-coverage: $(COVER_ROOT)
@printf $(COLOR) "Run integration tests with coverage..."
@go test $(INTEGRATION_TEST_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) $(INTEG_TEST_COVERPKG) -coverprofile=$(INTEGRATION_COVER_PROFILE)
@go test $(DB_TOOL_INTEGRATION_TEST_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) $(INTEG_TEST_COVERPKG) -coverprofile=$(DB_TOOL_COVER_PROFILE)

# TODO: rename it to functional-test
integration-test-coverage: $(COVER_ROOT)
@printf $(COLOR) "Run integration tests with coverage with $(PERSISTENCE_DRIVER) driver..."
@go test $(INTEG_TEST_ROOT) -timeout=$(TEST_TIMEOUT) -race $(TEST_TAG) -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER) $(INTEG_TEST_COVERPKG) -coverprofile=$(INTEG_COVER_PROFILE)

# TODO: rename it to functional-test
integration-test-xdc-coverage: $(COVER_ROOT)
@printf $(COLOR) "Run integration test for cross DC with coverage with $(PERSISTENCE_DRIVER) driver..."
@go test $(INTEG_TEST_XDC_ROOT) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER) $(INTEG_TEST_COVERPKG) -coverprofile=$(INTEG_XDC_COVER_PROFILE)

# TODO: rename it to functional-test
integration-test-ndc-coverage: $(COVER_ROOT)
@printf $(COLOR) "Run integration test for NDC with coverage with $(PERSISTENCE_DRIVER) driver..."
@go test $(INTEG_TEST_NDC_ROOT) -timeout=$(TEST_TIMEOUT) -race $(TEST_TAG) -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER) $(INTEG_TEST_COVERPKG) -coverprofile=$(INTEG_NDC_COVER_PROFILE)
Expand Down
59 changes: 0 additions & 59 deletions common/persistence/persistence-tests/cassandra_test.go
View file
Open in desktop

This file was deleted.

59 changes: 0 additions & 59 deletions common/persistence/persistence-tests/mysql_test.go
View file
Open in desktop

This file was deleted.

70 changes: 0 additions & 70 deletions common/persistence/persistence-tests/postgres_test.go
View file
Open in desktop

This file was deleted.

87 changes: 0 additions & 87 deletions common/persistence/persistence-tests/sqlite_test.go
View file
Open in desktop

This file was deleted.

0 ...tence/sql/sqlplugin/tests/context_test.go → ...ersistence/sql/sqlplugin/tests/context.go
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ...n/tests/history_current_execution_test.go → ...plugin/tests/history_current_execution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var (
}
)

func newHistoryCurrentExecutionSuite(
func NewHistoryCurrentExecutionSuite(
t *testing.T,
store sqlplugin.HistoryExecution,
) *historyCurrentExecutionSuite {
Expand Down
2 changes: 1 addition & 1 deletion ...sqlplugin/tests/history_execution_test.go → .../sql/sqlplugin/tests/history_execution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var (
testHistoryExecutionStateData = []byte("random history execution state data")
)

func newHistoryExecutionSuite(
func NewHistoryExecutionSuite(
t *testing.T,
store sqlplugin.HistoryExecution,
) *historyExecutionSuite {
Expand Down
Loading