imbajin commented on code in PR #687:
URL:
https://github.com/apache/incubator-hugegraph-toolchain/pull/687#discussion_r2480146866
##########
hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java:
##########
@@ -102,7 +102,7 @@ public void testAutoCreateSchema() {
"--batch-insert-threads", "2"
};
- HugeGraphLoader.main(args);
+ authmain(args);
Review Comment:
⚠️ **测试方法命名不清晰**
方法名 `authmain()` 含义不明确:
1. `auth` + `main` 还是 `auto` + `main`?
2. 没有说明它是干什么的
3. 与标准的 `main()` 方法容易混淆
建议使用更描述性的名称:
- `loadWithAuth(String[] args)`
- `executeLoaderWithAuthentication(String[] args)`
- 或者添加 Javadoc 解释其用途
另外,这个方法的实现在 diff 中没有显示,需要确认:
1. 它是否正确处理了认证逻辑
2. 是否有适当的错误处理
##########
hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java:
##########
@@ -246,15 +248,16 @@ public void testClearSchemaBeforeLoad() {
"josh,32,Beijing",
"peter,35,Shanghai",
"\"li,nary\",26,\"Wu,han\"");
- String[] args1 = new String[]{
+ List<String> argsList1 = new ArrayList<>(Arrays.asList(
"-f", structPath("clear_schema_before_load/struct.json"),
"-g", GRAPH,
"-h", SERVER,
"--batch-insert-threads", "2",
"--test-mode", "true"
- };
+ ));
+ argsList1.addAll(Arrays.asList("--username", "admin", "--password",
"pa"));
Review Comment:
⚠️ **不一致的认证参数传递模式**
在测试代码中发现了两种不同的认证参数传递方式:
**方式一(简单):** 直接调用 `authmain(args)`
```java
authmain(args);
```
**方式二(复杂):** 手动添加 username/password 到参数列表
```java
List<String> argsList = new ArrayList<>(Arrays.asList(...));
argsList.addAll(Arrays.asList("--username", "admin", "--password", "pa"));
HugeGraphLoader loader = new HugeGraphLoader(argsList.toArray(new
String[0]));
```
**问题:**
1. 为什么有些测试用方式一,有些用方式二?
2. 这会导致测试维护困难
**建议:**
统一使用一种方式,并提供辅助方法:
```java
private String[] withAuth(String... baseArgs) {
List<String> args = new ArrayList<>(Arrays.asList(baseArgs));
args.addAll(Arrays.asList("--username", "admin", "--password", "pa"));
return args.toArray(new String[0]);
}
```
##########
.github/workflows/loader-ci.yml:
##########
@@ -43,13 +43,13 @@ jobs:
fetch-depth: 2
- name: Install JDK 11
- uses: actions/setup-java@v3
+ uses: actions/setup-java@v4
Review Comment:
🧹 **Minor: GitHub Actions 版本更新很好,但缺少变更说明**
将 actions 从 v3 升级到 v4 是好的维护实践,但建议:
1. 在 PR 描述中说明升级原因(安全更新/新特性/弃用)
2. 确认 v4 的破坏性变更不会影响现有工作流
3. 考虑在单独的 PR 中进行依赖升级,便于回滚
参考:
- `actions/setup-java@v4`
- `actions/cache@v4`
- `actions/codecov-action@v4` (注意这个需要 token 配置)
--
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]
