Skip to content

test(bigquery-jdbc): add test tags & ability to skip them - #13916

Open
logachev wants to merge 4 commits into
mainfrom
kirl/tag_tests
Open

test(bigquery-jdbc): add test tags & ability to skip them#13916
logachev wants to merge 4 commits into
mainfrom
kirl/tag_tests

Conversation

@logachev

Copy link
Copy Markdown
Contributor

No description provided.

@logachev
logachev marked this pull request as ready for review July 27, 2026 22:45
@logachev
logachev requested review from a team as code owners July 27, 2026 22:45

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for excluding specific test groups or tags via an excludedGroups variable in the Makefile and annotates several integration tests with @Tag("advanced") and @Tag("known_issue"). The review feedback highlights multiple instances in the Makefile where excludedGroups is passed unquoted to $(MAKE) or within Docker arguments. If the variable contains spaces, this will cause word splitting and lead to build or execution failures; wrapping these references in quotes is recommended to ensure robust execution.


unit-test-coverage:
$(MAKE) unittest
$(MAKE) unittest excludedGroups=$(excludedGroups)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If excludedGroups contains spaces (e.g., excludedGroups="known_issue, advanced"), passing it unquoted to $(MAKE) will cause make to treat the words after the space as separate targets to build, leading to a build failure. Wrapping the variable in double quotes prevents this word splitting.

	$(MAKE) unittest excludedGroups="$(excludedGroups)"


full-coverage:
$(MAKE) integration-test skipSurefire=false test=ITBigQueryJDBCTest,ITNightlyBigQueryTest
$(MAKE) integration-test skipSurefire=false test=ITBigQueryJDBCTest,ITNightlyBigQueryTest excludedGroups=$(excludedGroups)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If excludedGroups contains spaces (e.g., excludedGroups="known_issue, advanced"), passing it unquoted to $(MAKE) will cause make to treat the words after the space as separate targets to build, leading to a build failure. Wrapping the variable in double quotes prevents this word splitting.

	$(MAKE) integration-test skipSurefire=false test=ITBigQueryJDBCTest,ITNightlyBigQueryTest excludedGroups="$(excludedGroups)"


docker-unittest: |
$(MAKE) .docker-run args="make unittest test=$(test)"
$(MAKE) .docker-run args="make unittest test=$(test) excludedGroups=$(excludedGroups)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If excludedGroups contains spaces (e.g., excludedGroups="known_issue, advanced"), passing it unquoted inside args will cause the shell to split the arguments when executing the docker command, leading to syntax or execution errors. Wrapping the variable in single quotes inside the double-quoted args string prevents this word splitting.

	$(MAKE) .docker-run args="make unittest test=$(test) excludedGroups='$(excludedGroups)'"


docker-integration-test: .check-env
$(MAKE) .docker-run args="make integration-test test=$(test) skipSurefire=$(skipSurefire)"
$(MAKE) .docker-run args="make integration-test test=$(test) skipSurefire=$(skipSurefire) excludedGroups=$(excludedGroups)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If excludedGroups contains spaces (e.g., excludedGroups="known_issue, advanced"), passing it unquoted inside args will cause the shell to split the arguments when executing the docker command, leading to syntax or execution errors. Wrapping the variable in single quotes inside the double-quoted args string prevents this word splitting.

	$(MAKE) .docker-run args="make integration-test test=$(test) skipSurefire=$(skipSurefire) excludedGroups='$(excludedGroups)'"

Comment on lines +162 to +163
$(MAKE) .docker-run args="make unit-test-coverage excludedGroups=$(excludedGroups)"
$(MAKE) .docker-run args="make full-coverage excludedGroups=$(excludedGroups)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If excludedGroups contains spaces (e.g., excludedGroups="known_issue, advanced"), passing it unquoted inside args will cause the shell to split the arguments when executing the docker command, leading to syntax or execution errors. Wrapping the variable in single quotes inside the double-quoted args string prevents this word splitting.

	$(MAKE) .docker-run args="make unit-test-coverage excludedGroups='$(excludedGroups)'"
	$(MAKE) .docker-run args="make full-coverage excludedGroups='$(excludedGroups)'"

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.

1 participant