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

Move test to AbstractTestHiveFileSystem #18172

Merged

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Jul 7, 2023

Description

The test testRecursiveDirectoriesWithSpecialCharacters() is being
moved to AbstractTestHiveFileSystem for being able to test it on
multiple object storage file systems.

Contains cherry-pick from #18167

Fixes #18169

Additional context and related issues

Release notes

( ) This is not user-visible or 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 Jul 7, 2023
@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from 9093996 to eb4f75a Compare July 7, 2023 13:02
@github-actions github-actions bot added tests:hive hive Hive connector labels Jul 7, 2023
@findinpath findinpath self-assigned this Jul 7, 2023
@findinpath findinpath requested review from findepi and pajaks July 10, 2023 08:27
@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from eb4f75a to 29762b5 Compare July 10, 2023 08:28
@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from 29762b5 to a0fbbaa Compare July 10, 2023 21:49
@findinpath findinpath requested a review from findepi July 10, 2023 21:49
.map(Path::new)
.toList();
// Should not include any hidden files, folders, or nested files
assertEqualsIgnoreOrder(shallowListing, ImmutableList.of());
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be good to have at least one non hidden file on this level to check if it's listed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In such a case, the file would pop up in the listing.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the idea is to test listening on top level.

assertEqualsIgnoreOrder(shallowListing, ImmutableList.of());
}

protected void createDirectory(String bucketName, String key)
Copy link
Member

Choose a reason for hiding this comment

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

This function is almost the same as createFile. Can reduce this duplication?

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 slightly modified createDirectory.
I find the duplication between them at the moment still small enough to warrant having two similar methods.

@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from 2c8ce54 to 32bb3ec Compare July 17, 2023 13:35
@findinpath findinpath requested a review from pajaks July 17, 2023 13:35
@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from 32bb3ec to d31d0fd Compare July 18, 2023 10:06
@findinpath
Copy link
Contributor Author

Unrelated CI failure in ci / artifacts-check

Error:  io.trino.server.rpm.ServerIT.testUninstall[/home/runner/work/trino/trino/core/trino-server-rpm/target/trino-server-rpm-423-SNAPSHOT.noarch.rpm, 19](2)  Time elapsed: 301.798 s  <<< FAILURE!
org.testcontainers.containers.ContainerLaunchException: Container startup failed for image eclipse-temurin:19-jre-centos7

@ebyhr
Copy link
Member

ebyhr commented Jul 26, 2023

/test-with-secrets sha=d31d0fdded87d2396b0988a3e15d84894892e6f4

@github-actions
Copy link

github-actions bot commented Jul 26, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5663986651

The test `testRecursiveDirectoriesWithSpecialCharacters()` is being
moved to `AbstractTestHiveFileSystem` for being able to test it on
multiple object storage file systems.
@findinpath findinpath force-pushed the findinpath/test-hive-recursive-listing branch from d31d0fd to dc43f19 Compare July 26, 2023 10:18
@findinpath
Copy link
Contributor Author

Failures: 
Error:    TestTrinoS3FileSystemAccessOperations.testSelectPartitionTable:142->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed Unable to load AWS credentials from any provider in the chain: [EnvironmentVariableCredentialsProvider: Unable to load AWS credentials from environment variables (AWS_ACCESS_KEY_ID (or AWS_ACCESS_KEY) and AWS_SECRET_KEY (or AWS_SECRET_ACCESS_KEY)), SystemPropertiesCredentialsProvider: Unable to load AWS credentials from Java system properties (aws.accessKeyId and aws.secretKey), WebIdentityTokenCredentialsProvider: You must specify a value for roleArn and roleSessionName, com.amazonaws.auth.profile.ProfileCredentialsProvider@1bac8cb2: profile file cannot be null, com.amazonaws.auth.EC2ContainerCredentialsProviderWrapper@48b80fc7: The requested metadata is not found at http://169.254.169.254/latest/meta-data/iam/security-credentials/]
Error:    TestTrinoS3FileSystemAccessOperations.testSelectPartitionTable:142->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed Unable to load AWS credentials from any provider in the chain: [EnvironmentVariableCredentialsProvider: Unable to load AWS credentials from environment variables (AWS_ACCESS_KEY_ID (or AWS_ACCESS_KEY) and AWS_SECRET_KEY (or AWS_SECRET_ACCESS_KEY)), SystemPropertiesCredentialsProvider: Unable to load AWS credentials from Java system properties (aws.accessKeyId and aws.secretKey), WebIdentityTokenCredentialsProvider: You must specify a value for roleArn and roleSessionName, com.amazonaws.auth.profile.ProfileCredentialsProvider@1bac8cb2: profile file cannot be null, com.amazonaws.auth.EC2ContainerCredentialsProviderWrapper@48b80fc7: The requested metadata is not found at http://169.254.169.254/latest/meta-data/iam/security-credentials/]
Error:    TestTrinoS3FileSystemAccessOperations.testSelectWithFilter:121->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed Unable to load AWS credentials from any provider in the chain: [EnvironmentVariableCredentialsProvider: Unable to load AWS credentials from environment variables (AWS_ACCESS_KEY_ID (or AWS_ACCESS_KEY) and AWS_SECRET_KEY (or AWS_SECRET_ACCESS_KEY)), SystemPropertiesCredentialsProvider: Unable to load AWS credentials from Java system properties (aws.accessKeyId and aws.secretKey), WebIdentityTokenCredentialsProvider: You must specify a value for roleArn and roleSessionName, com.amazonaws.auth.profile.ProfileCredentialsProvider@1bac8cb2: profile file cannot be null, com.amazonaws.auth.EC2ContainerCredentialsProviderWrapper@48b80fc7: The requested metadata is not found at http://169.254.169.254/latest/meta-data/iam/security-credentials/]
Error:    TestTrinoS3FileSystemAccessOperations.testSelectWithFilter:121->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed Unable to load AWS credentials from any provider in the chain: [EnvironmentVariableCredentialsProvider: Unable to load AWS credentials from environment variables (AWS_ACCESS_KEY_ID (or AWS_ACCESS_KEY) and AWS_SECRET_KEY (or AWS_SECRET_ACCESS_KEY)), SystemPropertiesCredentialsProvider: Unable to load AWS credentials from Java system properties (aws.accessKeyId and aws.secretKey), WebIdentityTokenCredentialsProvider: You must specify a value for roleArn and roleSessionName, com.amazonaws.auth.profile.ProfileCredentialsProvider@1bac8cb2: profile file cannot be null, com.amazonaws.auth.EC2ContainerCredentialsProviderWrapper@48b80fc7: The requested metadata is not found at http://169.254.169.254/latest/meta-data/iam/security-credentials/]

Failures unrelated to the current PR.
I can't reproduce them locally.

@ebyhr could you pls run again the PR with secrets?

@ebyhr
Copy link
Member

ebyhr commented Jul 26, 2023

/test-with-secrets sha=dc43f197824f4d8eb826903ea9c05987a3d6a2fc

@github-actions
Copy link

github-actions bot commented Jul 26, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5674482714

@findinpath
Copy link
Contributor Author

Unrelated failures in trino-iceberg and trino-iceberg (cloud-tests) in https://github.com/trinodb/trino/actions/runs/5674482714/job/15378926660

cc @ebyhr

@ebyhr ebyhr merged commit 2f513f6 into trinodb:master Jul 27, 2023
@github-actions github-actions bot added this to the 423 milestone Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed hive Hive connector
Development

Successfully merging this pull request may close these issues.

Add more test coverage for hive.recursive-directories functionality
4 participants