nevzheng commented on code in PR #13517: URL: https://github.com/apache/gravitino/pull/13517#discussion_r4263223963
########## dev/ci/core_test_identity.py: ########## @@ -0,0 +1,509 @@ +#!/usr/bin/env python3 +# 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. + +"""Build and reconcile normalized core-test identity manifests. + +Gradle writes one JUnit XML directory per Test task. This tool turns those +reports into stable, backend-neutral identity multisets so the H2, MySQL, and +PostgreSQL lanes can prove that they exercised the same test contract. It also +records status counts and elapsed test time for CI artifacts. +""" + +import argparse +from collections import Counter +from decimal import Decimal, InvalidOperation +import hashlib +import json +import math +from pathlib import Path +import re +import sys +import xml.etree.ElementTree as ET + + +SCHEMA_VERSION = 1 +LANES = ("unit", "h2", "mysql", "postgresql") +DATABASE_LANES = LANES[1:] +MANIFEST_LANES = LANES +STATUS_KEYS = ("passed", "skipped", "failures", "errors") + +BACKEND_NAME_PATTERN = r"h2|mysql|postgresql" +TEST_TEMPLATE_BACKEND_RE = re.compile( + rf"\[(?P<backend>{BACKEND_NAME_PATTERN})\s+Backend\]", re.IGNORECASE +) +STATS_BACKEND_CLASS_RE = re.compile( + rf"(?P<prefix>TestJdbcPartitionStatisticStorageIT)\$" + rf"(?P<backend>{BACKEND_NAME_PATTERN})Test(?=$|\$)", + re.IGNORECASE, +) +BACKEND_TOKEN_RE = re.compile( Review Comment: **note (non-blocking):** Closed as obsolete. https://github.com/apache/gravitino/pull/13517/commits/67975132b3f47b304c44dedf2f39c310f905e5c2 removes `dev/ci/core_test_identity.py` as part of the scope cut, so no lane parses testcase names any more. ########## .github/workflows/build.yml: ########## @@ -147,24 +151,26 @@ jobs: spark-connector/**/*.log build: - # The type of runner that the job will run on + name: build (${{ matrix.java-version }}, ${{ matrix.shard }}) Review Comment: **note (non-blocking):** Yes. Check names are in the description's "CI checks" section, and the merge order is on #13515. After the rebase this PR no longer carries #13495, so `build (17)` and the BackendIT names are unchanged; it adds `core-test (17, <lane>)` for the four lanes and `coverage`, and `build (17)` now skips `:core:test`. `.asf.yaml` names no required checks, so INFRA has nothing to change. The on-push/on-pr consolidation stays with #13495. ########## .github/workflows/build.yml: ########## @@ -190,12 +200,20 @@ jobs: exit 0 fi + shard_args_text="$(dev/ci/test-shards.sh build "${{ matrix.shard }}")" + mapfile -t shard_args <<< "${shard_args_text}" + + skip_docker_tests=false + case "${{ matrix.shard }}" in + core-unit|core-h2) skip_docker_tests=true ;; Review Comment: **note (non-blocking):** Keeping h2 without Docker, and the constraint now lives in [`CoreBackend`'s Javadoc](https://github.com/apache/gravitino/blob/4e8e79df0707a85257d999638a4311d5c874f7f1/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java#L36-L38) since the design doc is gone: the lane, not the class, decides whether Docker is needed, so a class tagged for H2 must not need a container itself. The identity-mismatch edge left with `reconcile`. ########## core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java: ########## @@ -121,19 +130,25 @@ public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContex "Running tests with H2 backend only. Set env var 'dockerTest=true' to include all backends."); } - return backendsToTest.stream().map(BackendInvocationContext::new); + return backendsToTest.stream() + .map(backendType -> new BackendInvocationContext(testMethodName, backendType)); } private static class BackendInvocationContext implements TestTemplateInvocationContext { + private final String testMethodName; private final String backendType; - public BackendInvocationContext(String backendType) { + public BackendInvocationContext(String testMethodName, String backendType) { + this.testMethodName = testMethodName; this.backendType = backendType; } @Override public String getDisplayName(int invocationIndex) { - return String.format("[%s Backend]", backendType.toUpperCase()); + // No trailing "()" here: @TestTemplate methods can declare parameters (e.g. an injected + // DatabaseTestContext), and a hardcoded empty parameter list would misrepresent the + // method's actual signature in JUnit XML/HTML reports. + return String.format("%s[%s Backend]", testMethodName, backendType.toUpperCase()); Review Comment: **chore (non-blocking):** Fixed in the PR description. "Does this PR introduce any user-facing change?" now says that core `@TestTemplate` cases are reported as `testMethod[H2 Backend]` instead of `[H2 Backend]`. Test history keyed on the old names starts fresh, exact `--tests` filters need the new name, and trailing-wildcard filters such as `*[H2 Backend]` still match. Nothing in the repository refers to the old names. The prefix stays even with the identity tool removed, because without it every case in a lane's report reads only `[H2 Backend]`. -- 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]
