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

Fix CI skip build and skip tests checks #54532

Merged
merged 18 commits into from
Sep 29, 2023
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 31 additions & 23 deletions tests/ci/pr_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,52 +304,60 @@ def has_changes_in_submodules(self):
return False

def can_skip_builds_and_use_version_from_master(self):
# TODO: See a broken loop
if FORCE_TESTS_LABEL in self.labels:
SmitaRKulkarni marked this conversation as resolved.
Show resolved Hide resolved
return False

if self.changed_files is None or not self.changed_files:
return False

for f in self.changed_files:
# TODO: this logic is broken, should be fixed before using
if (
not f.startswith("tests/queries")
or not f.startswith("tests/integration")
or not f.startswith("tests/performance")
):
return False

return True

def can_skip_integration_tests(self):
# TODO: See a broken loop
return not any(
f.startswith("programs")
or f.startswith("src")
or f.startswith("base")
or f.startswith("cmake")
or f.startswith("rust")
or f == "CMakeLists.txt"
or f == "tests/ci/build_check.py"
for f in self.changed_files
)

def can_skip_integration_tests(self, version):
SmitaRKulkarni marked this conversation as resolved.
Show resolved Hide resolved
if FORCE_TESTS_LABEL in self.labels:
return False

# If docker image(s) relevant to integration tests are updated
if self.sha not in version:
return False

if self.changed_files is None or not self.changed_files:
return False

if not self.can_skip_builds_and_use_version_from_master:
SmitaRKulkarni marked this conversation as resolved.
Show resolved Hide resolved
return False

# Integration tests can be skipped if only functional/performance tests are changes
for f in self.changed_files:
# TODO: this logic is broken, should be fixed before using
if not f.startswith("tests/queries") or not f.startswith(
if not f.startswith("tests/queries") and not f.startswith(
SmitaRKulkarni marked this conversation as resolved.
Show resolved Hide resolved
"tests/performance"
):
return False

return True

def can_skip_functional_tests(self):
# TODO: See a broken loop
def can_skip_functional_tests(self, version):
if FORCE_TESTS_LABEL in self.labels:
return False

# If docker image(s) relevant to functional tests are updated
if self.sha not in version:
Copy link
Member

Choose a reason for hiding this comment

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

It should be the other way around. If sha in the version, then the images are changed, so we run the tests

Copy link
Member Author

Choose a reason for hiding this comment

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

right, sorry it is fine in integration check, will fix it

return False

if self.changed_files is None or not self.changed_files:
return False

if not self.can_skip_builds_and_use_version_from_master:
return False

# Functional tests can be skipped if only integration/performance tests are changes
for f in self.changed_files:
# TODO: this logic is broken, should be fixed before using
if not f.startswith("tests/integration") or not f.startswith(
if not f.startswith("tests/integration") and not f.startswith(
Copy link
Member

Choose a reason for hiding this comment

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

No, it should be something more complex here.. We should check if the queries have changed. And split stateless and stateful tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, so if its not built again, then source is not changes . So we want to check if functional tests have been changed and if not we skip them.
and we don't want to check the tests/queries/bugs folder ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I couldn't quickly find what's in the bugs directory. According to git log it's the temporary jail-directory for broken tests. So we need to track only 0_stateless 1_stateful directories indeed

"tests/performance"
):
return False
Expand Down