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.

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

Rename host tests to functional tests #3782

Merged
merged 7 commits into from
Jan 11, 2023
Merged

Conversation

yux0
Copy link
Contributor

@yux0 yux0 commented Jan 6, 2023

What changed?
Rename host tests to functional tests

Why?
Categorize persistence integration tests and functional tests.

How did you test it?

Potential risks

Is hotfix candidate?

@yux0 yux0 marked this pull request as ready for review January 6, 2023 17:03
@yux0 yux0 requested a review from a team as a code owner January 6, 2023 17:03
Copy link
Contributor

@mindaugasrukas mindaugasrukas left a comment

Choose a reason for hiding this comment

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

LGTM

Makefile Outdated
@@ -262,32 +257,31 @@ build-tests:

unit-test: clean-test-results
@printf $(COLOR) "Run unit tests..."
@go test $(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) -race | tee -a test.log
@go test $(TEST_DIRS) -timeout=$(TEST_TIMEOUT) -tags=unittest -race | tee -a test.log
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add unittest build tag? I don't like the idea of mixing dir and tag based approaches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about I change everything to use build tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remove the unit test build tag and still use dir to filter tests. The problem is golang only support negative filtering. If I want to use the tag to filter integration test, I need to add !intergrationtest tag to all unit tests. I am not sure if there is other way around.

@@ -22,7 +22,9 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
Copy link
Member

Choose a reason for hiding this comment

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

These files need to be renamed too. You can do it later.

@yux0 yux0 merged commit 851c634 into temporalio:master Jan 11, 2023
@yux0 yux0 deleted the functional-test branch January 11, 2023 19:47
@@ -262,32 +260,31 @@ build-tests:

unit-test: clean-test-results
@printf $(COLOR) "Run unit tests..."
@go test $(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) $(TEST_TAG) -race | tee -a test.log
@go test $(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) $(TEST_DIRS) -race | tee -a test.log
Copy link
Member

@alexshtin alexshtin Jan 11, 2023

Choose a reason for hiding this comment

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

Suggested change
@go test $(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) $(TEST_DIRS) -race | tee -a test.log
@go test $(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) $(TEST_DIRS) $(TEST_TAG) -race | tee -a test.log

TEST_TAG is to pass custom test tags as parameter if needed.

@@ -296,27 +293,23 @@ $(COVER_ROOT):
unit-test-coverage: $(COVER_ROOT)
@printf $(COLOR) "Run unit tests with coverage..."
@echo "mode: atomic" > $(UNIT_COVER_PROFILE)
@go test ./$(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) -race -coverprofile=$(UNIT_COVER_PROFILE) || exit 1;
@go test ./$(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) -race -tags=unittest -coverprofile=$(UNIT_COVER_PROFILE) || exit 1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@go test ./$(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) -race -tags=unittest -coverprofile=$(UNIT_COVER_PROFILE) || exit 1;
@go test ./$(UNIT_TEST_DIRS) -timeout=$(TEST_TIMEOUT) -race -coverprofile=$(UNIT_COVER_PROFILE) || exit 1;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants