imbajin commented on code in PR #3169:
URL: https://github.com/apache/hugegraph/pull/3169#discussion_r3845844347
##########
.github/workflows/pd-store-ci.yml:
##########
@@ -146,9 +149,10 @@ jobs:
mvn test -pl hugegraph-pd/hg-pd-test -am -P pd-rest-test
- name: Upload coverage to Codecov
- uses: codecov/[email protected]
+ uses: codecov/codecov-action@v5
with:
- file: ${{ env.REPORT_DIR }}/*.xml
+ token: ${{ secrets.CODECOV_TOKEN }}
+ files: ${{ env.REPORT_DIR }}/*.xml
Review Comment:
‼️ Blocking: yes. Summary: This PD/Store input must be rebased with #3161's
aggregate report path. Evidence: #3161 now defines and validates `${{
env.REPORT_FILE }}` under the PD/Store test modules, while exact-head jobs
97408972447 and 97408972425 logged `not_found_files:
["target/site/jacoco/*.xml"]` and then discovered reports from multiple
modules. Resolve the conflict by using `${{ env.REPORT_FILE }}` here and set
`disable_search: true`; retain #3161's JaCoCo validator step as well.
##########
.github/workflows/commons-ci.yml:
##########
@@ -58,6 +58,7 @@ jobs:
mvn test -pl hugegraph-commons/hugegraph-rpc -Dtest=UnitTestSuite
-DskipCommonsTests=false
- name: Upload coverage to Codecov
- uses: codecov/[email protected]
+ uses: codecov/codecov-action@v5
with:
- file: target/jacoco.xml
+ token: ${{ secrets.CODECOV_TOKEN }}
+ files: target/jacoco.xml
Review Comment:
‼️ Blocking: yes. Summary: The explicit Commons report path is incorrect and
the default search masks the failure. Evidence: exact-head job 97406468118
reported `not_found_files: ["target/jacoco.xml"]` and then uploaded four
reports, including PD, Store, and Server XMLs. Use
`hugegraph-commons/target/jacoco.xml` and set `disable_search: true` so this
job uploads only the Commons report.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-codecov-upload-config.sh:
##########
@@ -0,0 +1,182 @@
+#!/bin/bash
+#
+# 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.
+#
+
+set -euo pipefail
+
+SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+REPO_ROOT=$(cd "${SCRIPT_DIR}/../../../../.." && pwd)
+
+python3 - "${REPO_ROOT}" <<'PY'
+import pathlib
+import re
+import sys
+
+repo_root = pathlib.Path(sys.argv[1])
+expected_uploads = {
+ ".github/workflows/commons-ci.yml": 1,
+ ".github/workflows/pd-store-ci.yml": 3,
+ ".github/workflows/server-ci.yml": 1,
+}
+action_pattern = re.compile(
+ r"^(?P<indent>\s*)(?P<dash>-\s+)?uses:\s*"
+ r"codecov/codecov-action@(?P<version>\S+)\s*$"
+)
+version_pattern = re.compile(r"^v(?P<major>\d+)(?:[.-].*)?$")
+errors = []
+workflow_dir = repo_root / ".github/workflows"
+workflow_paths = sorted(
+ set(workflow_dir.glob("*.yml")) | set(workflow_dir.glob("*.yaml"))
+)
+checked_expected_workflows = set()
+
+
+def indentation(line):
+ return len(line) - len(line.lstrip())
+
+
+def find_uploads(lines):
+ uploads = []
+
+ for line_number, line in enumerate(lines, start=1):
+ match = action_pattern.match(line)
+ if match is None:
+ continue
+
+ uses_indent = len(match.group("indent"))
+ if match.group("dash") is not None:
+ uses_indent += len(match.group("dash"))
+ step_indent = uses_indent - 2
+ block = []
+ for candidate in lines[line_number:]:
+ if candidate.strip() and indentation(candidate) <= step_indent:
+ break
+ block.append(candidate)
+ uploads.append(
+ (line_number, match.group("version"), block, uses_indent)
+ )
+
+ return uploads
+
+
+def read_inputs(block, uses_indent):
+ inputs = {}
+ in_with_block = False
+ for candidate in block:
+ candidate_indent = indentation(candidate)
+ if candidate_indent == uses_indent and candidate.strip() == "with:":
+ in_with_block = True
+ continue
+ if (in_with_block and candidate.strip() and
+ candidate_indent <= uses_indent):
+ break
+ if not in_with_block or candidate_indent != uses_indent + 2:
+ continue
+ candidate_match = re.match(
+ r"^\s*(?P<key>[a-zA-Z_]+):\s*(?P<value>.*?)\s*$",
+ candidate,
+ )
+ if candidate_match is not None:
+ inputs[candidate_match.group("key")] =
candidate_match.group("value")
+ return inputs
+
+
+def parse_uploads(lines):
+ return [
+ (line_number, version, read_inputs(block, uses_indent))
+ for line_number, version, block, uses_indent in find_uploads(lines)
+ ]
+
+
+def check_mixed_upload_indentation():
+ lines = [
+ "jobs:",
+ " first:",
+ " steps:",
+ " - uses: codecov/codecov-action@v5",
+ " with:",
+ " token: ${{ secrets.CODECOV_TOKEN }}",
+ " files: first.xml",
+ " second:",
+ " steps:",
+ " - uses: codecov/codecov-action@v5",
+ " with:",
+ " token: ${{ secrets.CODECOV_TOKEN }}",
+ " files: second.xml",
+ ]
+ expected_inputs = [
+ {
+ "token": "${{ secrets.CODECOV_TOKEN }}",
+ "files": "first.xml",
+ },
+ {
+ "token": "${{ secrets.CODECOV_TOKEN }}",
+ "files": "second.xml",
+ },
+ ]
+ actual_inputs = [inputs for _, _, inputs in parse_uploads(lines)]
+ if actual_inputs != expected_inputs:
+ return ["Codecov uploads with mixed indentation were parsed
incorrectly"]
+ return []
+
+
+errors.extend(check_mixed_upload_indentation())
+
+
+for workflow_path in workflow_paths:
+ relative_path = str(workflow_path.relative_to(repo_root))
+ lines = workflow_path.read_text(encoding="utf-8").splitlines()
+ uploads = parse_uploads(lines)
+
+ expected_count = expected_uploads.get(relative_path)
+ if expected_count is not None and len(uploads) != expected_count:
+ errors.append(
+ f"{relative_path}: expected {expected_count} Codecov uploads, "
+ f"found {len(uploads)}"
+ )
+ if expected_count is not None:
+ checked_expected_workflows.add(relative_path)
+
+ for line_number, version, inputs in uploads:
+ version_match = version_pattern.match(version)
+ if version_match is None or int(version_match.group("major")) < 5:
+ errors.append(
+ f"{relative_path}:{line_number}: Codecov action {version} "
+ "uses the legacy uploader"
+ )
+
+ if not inputs.get("files"):
Review Comment:
⚠️ Blocking: no. Summary: The new contract test does not guard the
report-selection behavior this change depends on. Evidence: it checks only
action count/version, `files`, and token; the exact-head CI still passed the
configuration test while Codecov reported missing explicit files and
auto-discovered unrelated reports. Extend the contract to assert each
workflow's intended report path and `disable_search: true`, while keeping
#3161's report-content validation separate.
--
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]