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

Fixed Functional Test Failures #24123

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

abevk2023
Copy link
Contributor

Resolves following failures in functional tests.

% ./mvnw test -pl presto-native-execution \
            -Dtest=com.facebook.presto.nativeworker.TestPrestoContainerBasicQueries \
            -DcoordinatorImage="presto-coordinator:latest" \
            -DworkerImage="presto-worker:latest"
[ERROR] Failures:
[ERROR] com.facebook.presto.nativeworker.TestPrestoContainerTpchConnectorQueries.init  
 java.io.IOException: File exists: testcontainers/coordinator/etc/catalog/tpch.properties
at com.facebook.presto.nativeworker.ContainerQueryRunnerUtils.createPropertiesFile(ContainerQueryRunnerUtils.java:208)
[ERROR]   TestPrestoContainerBasicQueries.testArraySortFunction:74->AbstractTestQueryFramework.assertQuery:174 Execution of 'actual' query failed: SELECT array_sort(ARRAY [5, 20, null, 5, 3, 50])
[ERROR]   TestPrestoContainerBasicQueries.testPresenceAndBasicFunctionality:37->AbstractTestQueryFramework.computeActual:134->AbstractTestQueryFramework.computeActual:139 » Runtime java.sql.SQLException: ResultSet is closed
[INFO]
[ERROR] Tests run: 4, Failures: 2, Errors: 0, Skipped: 0
== NO RELEASE NOTE ==

@abevk2023 abevk2023 requested a review from a team as a code owner November 22, 2024 16:24
@abevk2023 abevk2023 marked this pull request as draft November 22, 2024 16:24
@abevk2023 abevk2023 marked this pull request as ready for review November 25, 2024 08:21
}

try (OutputStream output = new FileOutputStream(file);
try (OutputStream output = new FileOutputStream(file, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
try (OutputStream output = new FileOutputStream(file, false);
try (OutputStream output = new FileOutputStream(file);

Aren't these equivalent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Corrected.

String arrayElementTypeName = metaData.getColumnTypeName(i);
Type elementType = mapSqlTypeNameToType(arrayElementTypeName);
types.add(new ArrayType(elementType));
if (typeName.startsWith("ARRAY(") && typeName.endsWith(")")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use TypeSignature.parseTypeSignature to retrieve the base type name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Done

@abevk2023 abevk2023 force-pushed the fix_functional_tests branch from e4f9256 to affbf8c Compare December 13, 2024 06:08
@tdcmeehan tdcmeehan added the from:IBM PR from IBM label Dec 13, 2024
@prestodb-ci prestodb-ci requested review from a team, bibith4 and namya28 and removed request for a team December 13, 2024 15:18
@prestodb-ci
Copy link

Saved that user @abevk2023 is from IBM

@@ -244,6 +244,12 @@
<artifactId>presto-jdbc</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid this new dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This dependency is required for the MountableFile. Without this, there is runtime error while starting container.

java.lang.NoClassDefFoundError: org/apache/commons/lang3/SystemProperties

	at org.apache.commons.compress.archivers.tar.TarArchiveEntry.normalizeFileName(TarArchiveEntry.java:231)
	at org.apache.commons.compress.archivers.tar.TarArchiveEntry.<init>(TarArchiveEntry.java:490)
	at org.testcontainers.utility.MountableFile.recursiveTar(MountableFile.java:355)
	at org.testcontainers.utility.MountableFile.transferTo(MountableFile.java:327)
	at org.testcontainers.containers.ContainerState.copyFileToContainer(ContainerState.java:306)
	at org.testcontainers.containers.ContainerState.copyFileToContainer(ContainerState.java:282)
	at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:430)
	at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:344)
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:334)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:322)
	at com.facebook.presto.nativeworker.ContainerQueryRunner.<init>(ContainerQueryRunner.java:97)

@tdcmeehan tdcmeehan merged commit 33d7c03 into prestodb:master Dec 16, 2024
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:IBM PR from IBM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants