bitflicker64 commented on code in PR #791:
URL:
https://github.com/apache/hugegraph-toolchain/pull/791#discussion_r4199144081
##########
.github/scripts/test_candidate_sdk.py:
##########
@@ -15,19 +15,95 @@
# License for the specific language governing permissions and limitations
# under the License.
#
-"""Bounded SDK provenance fixtures; these do not build real Maven artifacts."""
+"""SDK provenance fixtures and an opt-in real Maven parent-metadata
regression."""
import copy
import json
+import os
from pathlib import Path
import shutil
import subprocess
import tempfile
import unittest
+import uuid
+import xml.etree.ElementTree as ET
import verify_candidate_image_sdk as sdk
+class CandidateParentModelTest(unittest.TestCase):
+ @unittest.skipUnless(os.environ.get("CANDIDATE_SDK_MAVEN_TEST"),
Review Comment:
Important: `CandidateParentModelTest` is the regression test for the new
flatten step in `install-candidate-sdk.sh`, but no workflow sets
`CANDIDATE_SDK_MAVEN_TEST`, so CI always skips it. `git grep
CANDIDATE_SDK_MAVEN_TEST 9a63a4c` finds only this file. The `plan` job in
`ci.yml` runs `python3 -m unittest discover -s .github/scripts -p 'test_*.py'`
with no such variable set, and locally that discover reports
`test_license_metadata_can_rebuild_installed_root_parent ... skipped`.
`server-fixture-ci.yml` runs `test_candidate_sdk.py` the same way. The
`install-server.test.cjs` check only compares the argument list, so if the
flatten step stops working, CI stays green.
When I enabled it locally with Maven 3.9.16, the test passed in about 6 s.
It also wrote two `org/example/candidatesdk/g<uuid>` groups into the default
`~/.m2/repository`, because `run()` sets `--settings` but not
`-Dmaven.repo.local`. That is the opposite of what the comment on the next line
says ("never the user's SDK artifacts").
Requested change: run it in CI, for example by setting
`CANDIDATE_SDK_MAVEN_TEST: mvn` on the plan job's "Test CI policy and retry
guards" step (Maven is on `ubuntu-latest`). Also pass
`-Dmaven.repo.local=<temporary>/m2` in `run()` so the test never writes to the
caller's local repository.
##########
docs/ci.md:
##########
@@ -69,30 +104,40 @@ selected consumers again. Source, type definitions, tests
and CI configuration
are never treated as docs. The gate checks the current workflow's actual
selected
job results and fixture producers. Its result report records the run, attempt
and
input metadata for diagnostics; it is not a reusable success receipt.
-The planner determines affected consumers before resolving external Server
inputs. Plans that require Server fixtures record both the historical release
and the locked candidate source; Go-only module tests consume only the release
fixture. Image-only and plain documentation plans need neither baseline.
Unverifiable selection conservatively resolves both. A failed required baseline
lookup fails planning; without a published plan, recovery requires a full
workflow rerun.
+The planner determines affected consumers before resolving external Server
inputs.
+Plans that require Server fixtures record the historical release and locked
+candidate source; Go-only module tests consume only the release fixture.
Image-only
+and plain documentation plans need neither baseline. Unverifiable selection
+conservatively resolves both. A failed selected baseline lookup fails planning;
+without a published plan, recovery requires a full workflow rerun.
PR planning records the event's head, base and source before querying live
metadata.
-A known mismatch with the checkout merge or current PR fails planning;
expanding
-coverage cannot make an outdated checkout current. After all selected jobs and
-fixtures succeed, the gate rechecks the open PR's head, base and source using a
-read-only API request. Changed inputs or unavailable metadata fail the gate.
-Refresh the branch and start a new PR event after head or base movement: GitHub
-reruns retain the original commit and event, so rerunning alone cannot refresh
the
-base. An unknown API or selection failure falls back to full coverage; a manual
-full rerun can recover a transient failure while the recorded inputs remain
current.
-
-CodeQL follows the module gate and retains its weekly scan. During migration,
-`Analyze (java)` still runs on every PR, after a successful module gate.
-The legacy alias is PR-only; documentation pushes do not force scanning or fail
-because a deliberately unselected scan was skipped. Scan write permissions are
-limited to the security job. After ASF branch protection actually
-requires `check-license-header` and `affected-module-tests`, an administrator
can
-set `CI_OPTIMIZED_REQUIRED=true` to skip security scans for documentation-only
-changes. This switch is disabled by default and does not alter branch
protection.
+A known mismatch with the checkout merge, current head or source fails
planning.
+The report records the actual tested base and merge; advancing master alone
does
+not invalidate a completed result, consistent with non-strict branch
protection.
+Unknown selection falls back to full validation. No cross-run success is
reused.
+
+CodeQL starts independently on affected PR and push inputs, with its original
+analysis identity and weekly full scan. Java, frontend and Python changes
select
+their supported scan language; shared/unknown build inputs keep all three
languages.
+Go-only and image-only changes do not start unrelated source scans. Manual and
+weekly runs scan all three languages. Plain prose-only PRs skip compilation,
+fixtures, images and scanning; native workflows retain lightweight selection
jobs.
+Packaged README files and shared build/configuration inputs are not plain
prose.
+The `CI_OPTIMIZED_REQUIRED` variable is no longer needed.
+
+The dependency inventory audit includes root `.mvn/**`, POMs, dependency
manifests,
+assembly/license inputs and staged Maven settings. It runs from the repository
root
+with the same root Maven configuration, separately from the required license
check.
+
+For rollout, `.asf.yaml` requests only `check-license-header`. Confirm actual
branch
+protection has reconciled before relying on advisory cancellation. The existing
+module summary and native `Analyze (java)` context remain available during the
Review Comment:
Minor: this says the native `Analyze (java)` context "remain[s] available
during the migration", but the PR removes the `legacy-java-analysis` job that
produced it on every PR. `Analyze (java)` now comes only from
`codeql-analysis.yml`, and that job runs only when `scan_languages()` selects
`java`. A PR that touches only docs, Python, frontend, Go or image files gets
no `Analyze (java)` check (see the
`test_codeql_uses_affected_supported_languages` cases). Live master protection
requires only `check-license-header` and `affected-module-tests`, so nothing
blocks today. Still, anyone who relies on this sentence will look for a context
that does not exist.
Requested change: say that `Analyze (java)` runs only when Java or shared
build inputs change. If a stable context is really needed during the rollout,
keep a lightweight job that always reports it.
--
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]
