Skip to content

Do not fuzz tests that require S3#43302

Closed
serxa wants to merge 1 commit intomasterfrom
do-not-fuzz-s3
Closed

Do not fuzz tests that require S3#43302
serxa wants to merge 1 commit intomasterfrom
do-not-fuzz-s3

Conversation

@serxa
Copy link
Copy Markdown
Member

@serxa serxa commented Nov 16, 2022

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

...

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Nov 16, 2022
@tavplubix
Copy link
Copy Markdown
Member

Do not fuzz tests that require S3

Why? Queries that try to access S3 will just fail, it's not a problem. Unavailability of S3 should not crash the server

# echo ${path//.sql.j2/.gen.sql}
# but it doesn't allow to use regex
echo "$path" | sed 's/\.sql\.j2$/.gen.sql/'
if grep '^-- Tag' "$path" | grep "needs s3" >/dev/null 2>&1; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do I understand correctly that it looks for a line like the following?

-- Tags: no-fasttest
-- Tag no-fasttest: needs s3

The comment on the second line is not mandatory, it has no effect and can be omitted. Moreover, it may contain arbitrary text like "requires s3" or "needs minio" or whatever instead of "needs s3".

@tavplubix tavplubix self-assigned this Nov 16, 2022
@serxa serxa closed this Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants