bitflicker64 commented on code in PR #3261: URL: https://github.com/apache/hugegraph/pull/3261#discussion_r4176466434
########## hugegraph-store/hg-store-node/src/test/java/org/apache/hugegraph/store/node/controller/FixGraphIdControllerTest.java: ########## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.store.node.controller; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.hugegraph.store.util.HgStoreException; +import org.junit.Assert; +import org.junit.Test; + +public class FixGraphIdControllerTest { Review Comment: Important: This test class never runs in CI, so the new checks in `FixGraphIdController.updateGraphId` have no test that runs. The tests here use JUnit 4 (`org.junit.Test`). `hg-store-node` pulls in `spring-boot-starter-test`, Surefire 3.5.6 picks `JUnitPlatformProvider`, and there is no vintage engine. In the store job at this head (run 37172218875), every `surefire:3.5.6:test (default-test) @ hg-store-node` reports `Tests run: 0`. No workflow targets `hg-store-node` tests directly either: the store lanes only run `mvn test -pl hugegraph-store/hg-store-test -am -P store-*-test`, and those profiles select suites by include pattern. `GraphIDManagerTest` does run because it was added to the `store-core-test` includes. This class was not. So a regression in the controller wiring, for example calling `updateGraphIds` before `checkGraphIds` or dropping the null/duplicate check, would still pass CI. Requested change: move these cases into `hg-store-test` (it already depends on `hg-store-node`) and add the class to the `store-core-test` includes next to `GraphIDManagerTest`. Or add `junit-vintage-engine` to `hg-store-node` test scope and wire a CI step that runs the module's tests. Then confirm that the surefire report for `FixGraphIdControllerTest` appears in the store job. ########## .asf.yaml: ########## @@ -41,8 +41,7 @@ github: - Analyze (java) - CodeQL - check-license - - build-server (memory, 11) - - build-commons (11) + - build-server (memory, 17) Review Comment: Minor: This removes `build-commons (11)` from the required checks but does not add `build-commons (17)`, so Commons test failures no longer block merges to master. `commons-ci.yml` still runs on every `pull_request` with no path filter, and at this head it reports the check as `build-commons (17)`. Only the server check was renamed to 17. The Commons unit and RPC suites were a merge gate before this PR and are no longer one after it. That matters more in this series, because this PR also changes the Commons compiler target and dependency versions (commons-configuration2 2.10.1, commons-lang3 3.18.0). Requested change: add `- build-commons (17)` under `contexts`, or say in the PR description why Commons is being dropped as a merge gate. -- 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]
