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 tests failing due to lowering restrictions on s3 paths #19361

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

homar
Copy link
Member

@homar homar commented Oct 11, 2023

Description

Fix tests broken in 979e54a

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Oct 11, 2023
@github-actions github-actions bot added tests:hive iceberg Iceberg connector delta-lake Delta Lake connector hive Hive connector labels Oct 11, 2023
@homar homar force-pushed the homar/fix_failing_s3_related_tests branch 2 times, most recently from dbe9686 to ddbd438 Compare October 12, 2023 07:28
@homar homar marked this pull request as ready for review October 12, 2023 09:07
@@ -137,7 +137,7 @@ public void testBasicOperationsWithProvidedSchemaLocation(boolean partitioned, L
assertUpdate("CREATE TABLE " + qualifiedTableName + "(col_int int, col_str varchar)" + partitionQueryPart);
try (UncheckedCloseable ignoredDropTable = onClose("DROP TABLE " + qualifiedTableName)) {
// in case of regular CREATE TABLE, location has generated suffix
String expectedTableLocationPattern = (schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName + "-[a-z0-9]+";
String expectedTableLocationPattern = Pattern.quote(schemaLocation.endsWith("/") ? schemaLocation : schemaLocation + "/") + tableName + "-[a-z0-9]+";
Copy link
Member

Choose a reason for hiding this comment

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

why not quote tableName too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think it was necessary, I may add it but it is only in our tests and currently they work without it

@findepi findepi merged commit 986e3ca into trinodb:master Oct 12, 2023
225 of 228 checks passed
@github-actions github-actions bot added this to the 430 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed delta-lake Delta Lake connector hive Hive connector iceberg Iceberg connector
Development

Successfully merging this pull request may close these issues.

2 participants