steveloughran commented on code in PR #8048:
URL: https://github.com/apache/hadoop/pull/8048#discussion_r2455036391
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java:
##########
@@ -37,10 +46,33 @@
*/
public class ITestS3AFileSystemIsolatedClassloader extends AbstractS3ATestBase
{
+ private static String customClassName = "custom.class.name";
+
+ private static class CustomCredentialsProvider implements
AwsCredentialsProvider {
+
+ public CustomCredentialsProvider() {
+ }
+
+ @Override
+ public AwsCredentials resolveCredentials() {
+ return null;
+ }
+
+ }
+
private static class CustomClassLoader extends ClassLoader {
}
- private final ClassLoader customClassLoader = new CustomClassLoader();
+ private final ClassLoader customClassLoader = spy(new CustomClassLoader());
+ {
+ try {
Review Comment:
this is a nice way to simulate classloader pain.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java:
##########
@@ -28,6 +29,14 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.s3a.impl.InstantiationIOException;
+
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
Review Comment:
nit: put the amazon imports in the same group as the junit ones
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java:
##########
@@ -100,11 +122,26 @@ public void defaultIsolatedClassloader() throws
IOException {
.isEqualTo(fs.getClass().getClassLoader())
.describedAs("the classloader that loaded the fs");
});
+
+ Throwable thrown = Assertions.catchThrowable(() -> {
Review Comment:
Use our `LambdaTestUtils.intercept()`; it's like the spark one and does the
casting checks
```
InstantiationIOException ex = intercept(InstantiationIOException.class, ()
-> { assert...})
```
we have a `assertExceptionContains` to look at the inner stuff, but the
assert of L136 is fine.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java:
##########
@@ -115,11 +152,31 @@ public void isolatedClassloader() throws IOException {
.isEqualTo(fs.getClass().getClassLoader())
.describedAs("the classloader that loaded the fs");
});
+
+ Throwable thrown = Assertions.catchThrowable(() -> {
Review Comment:
again `intercept()` and cut the assert at L163
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java:
##########
@@ -77,19 +109,9 @@ private void assertInNewFilesystem(Map<String, String>
confToSet, Consumer<FileS
}
}
- private Map<String, String> mapOf() {
- return new HashMap<>();
- }
-
- private Map<String, String> mapOf(String key, String value) {
- HashMap<String, String> m = new HashMap<>();
- m.put(key, value);
- return m;
- }
Review Comment:
It's because we only switched to java17 yesterday! And in trunk only.
If you want to see this change in Hadoop 3.4.3 it'll still need to be java8
code, so this needs to be restored. Otherwise: trunk/3.5.0 only
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
