slfan1989 commented on PR #7371:
URL: https://github.com/apache/hadoop/pull/7371#issuecomment-2651576963
> Sorry that I am new to `jupiter.api.extension`. So I am not able to review
those changes. Please see if someone else could review them. If not, I suggest
separating them to another PR and commit the other changes first.
>
> +1 the other changes looks good.
Thank you for your message! After carefully reviewing the code, I found that
we may not be able to split it into two PRs. The main reason is that most of
the HttpFS unit tests inherit from `HTestCase`, and a key detail is that all
the tests rely on `TestDirHelper`, `TestJettyHelper`, and `TestHdfsHelper` to
set up the foundational test environment before executing the actual methods.
These three methods are the core parts that we have rewritten in this case.
Let me briefly describe the purpose of these methods.
> BeforeEachCallback
The BeforeEachCallback interface allows us to perform custom actions before
each test method is executed. It is part of JUnit 5's extension model and can
be used to perform initialization tasks before each test method runs.
> AfterEachCallback
The AfterEachCallback interface allows us to perform custom actions after
each test method has been executed. It is commonly used for cleaning up
resources, resetting states, or performing other tasks.
> TestExecutionExceptionHandler
The TestExecutionExceptionHandler interface allows us to handle exceptions
thrown during the execution of a test method. You can use it to customize the
handling of exceptions, allowing certain exceptions to be suppressed or
transformed into other types of exceptions.
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/test/TestHdfsHelper.java
Let's take `TestHdfsHelper.java` as an example.
The core logic of JUnit 4.
```
@Override
public void evaluate() throws Throwable {
Configuration conf = HadoopUsersConfTestHelper.getBaseConf();
if (Boolean.parseBoolean(System.getProperty(HADOOP_MINI_HDFS, "true"))) {
miniHdfs = startMiniHdfs(conf);
conf = miniHdfs.getConfiguration(0);
}
try {
// Step1. Before executing the unit test, initialize some configurations
(Conf) and directories.
HDFS_CONF_TL.set(conf);
HDFS_TEST_DIR_TL.set(resetHdfsTestDir(conf));
// Step2. Execute the unit test.
statement.evaluate();
} finally {
// Step3. After the unit test finishes, clean up the configurations
(Conf) and directories.
HDFS_CONF_TL.remove();
HDFS_TEST_DIR_TL.remove();
}
}
```
In JUnit 5, we execute Step 1 in `BeforeEach` and Step 3 in `AfterEach`.
Overall, this makes the code clearer.
> Step1. beforeEach
```
@Override
public void beforeEach(ExtensionContext context) throws Exception {
super.beforeEach(context);
Method testMethod = context.getRequiredTestMethod();
TestHdfs testHdfsAnnotation = testMethod.getAnnotation(TestHdfs.class);
if (testHdfsAnnotation != null) {
this.statement = new HdfsStatement(testMethod.getName());
this.statement.evaluate();
}
}
public void evaluate() throws Exception {
Configuration conf = HadoopUsersConfTestHelper.getBaseConf();
if (Boolean.parseBoolean(System.getProperty(HADOOP_MINI_HDFS, "true"))) {
miniHdfs = startMiniHdfs(conf);
conf = miniHdfs.getConfiguration(0);
}
HDFS_CONF_TL.set(conf);
HDFS_TEST_DIR_TL.set(resetHdfsTestDir(conf));
}
```
> Step3. afterEach
```
@Override
public void afterEach(ExtensionContext extensionContext) throws Exception {
super.afterEach(extensionContext);
HDFS_CONF_TL.remove();
HDFS_TEST_DIR_TL.remove();
}
```
The purpose of `TestExecutionExceptionHandler` is to check whether the
thrown exception and message after a unit test fails match the expected ones.
It is an additional feature that does not affect the execution of the unit test.
Overall, we haven't changed the existing execution framework; instead, we've
optimized some details. The new changes do not impact the original unit tests,
so I believe the risk is manageable.
--
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]
