Files
f929d68ba6 test: clear the ruff baseline in tests/ so the lint gate reflects the PR under review (#52115)
issue: #52114

Makes `ruff check` and `ruff format --check` pass over all of `tests/`,
so the `Ruff (changed files only)` job stops failing PRs for violations
they did not introduce (most recently #51903, which got 69 errors from
an 8-line change).

Almost all of this is mechanical. The behavioural changes are called out
individually below: one `NameError` fix, three shadowed duplicate
definitions, 13 assertion gaps, and two undefined names that only became
visible once star imports were expanded.

### Layout

Seven reviewable commits:

| commit | what | files |
|---|---|---|
| 1 | `ruff format` | 170 |
| 2 | `ruff check --fix` (safe fixes only, no `--unsafe-fixes`) | 156 |
| 3 | F821 fix + `tests/ruff.toml` | 4 |
| 4 | three shadowed duplicate definitions | 4 |
| 5 | close 13 assertion gaps behind F841 | 7 |
| 6 | expand star imports, re-enable F403/F405/F821 | 33 |
| 7 | enable E402, E712, E722 | 16 |

1745 violations → 0. Only `tests/` is touched.

### The undefined-name bug

`chaos/test_chaos_memory_stress.py`:
`TestMemoryStressReplica.prepare_collection` referenced bare `nb` and
`dim`. Those are class attributes, never module-level names, so the
fixture raised `NameError` the moment it ran — and the line directly
above it already used `self.dim`. Changed to `self.nb` / `self.dim`.
This was the only F821 in `tests/`; **the rule stays enabled**.

### Three shadowed duplicate definitions

Three places define the same name twice in the same scope. Python keeps
the later one, so the earlier definition is dead while still reading as
live code — a test that looks like it exists and never runs.

**`restful_client/testcases/test_vector_operations.py` —
`TestDeleteVector`**

```python
@pytest.mark.L0
class TestDeleteVector(TestBase):          # line 778 — shadowed, never ran
    def test_delete_vector_default(...)    # 2x2 parametrized

@pytest.mark.L1
class TestDeleteVector(TestBase):          # line 838 — wins
    def test_delete_vector_with_invalid_api_key(...)
    def test_delete_vector_with_invalid_collection_name(...)
    def test_delete_vector_with_non_primary_key(...)
```

The second holds only negative cases, and the file already uses
`TestInsertVectorNegative` / `TestSearchVectorNegative`, so this is a
copy whose class name was never updated. Renamed it to
`TestDeleteVectorNegative`.

**This is the one to look at:** it brings back an `L0` case, so 4
parametrizations of `test_delete_vector_default` start running in PR CI.
They have never executed, so if the restful delete path has drifted
since they were written, this is where it surfaces.

**`python_client/testcases/test_compaction.py` —
`test_compact_after_delete`**

Defined twice in `TestCompactionParams`, covering different things: the
shadowed one (L3, `@parametrize("delete_pos", [1, tmp_nb // 2])`) covers
*partial* deletes plus a compaction-plan check; the surviving one (L2)
deletes everything. There is already a separate
`test_compact_after_delete_all`, so partial-delete coverage existed only
in the shadowed case and was not running. Renamed it to
`test_compact_after_delete_partial`; the surviving definition is
untouched.

**`python_client/utils/util_pymilvus.py` — `add_vector_field`**

Defined twice with different signatures:

```python
def add_vector_field(entities, is_normal=False):     # line 455 — shadowed
    ...
    entities.append(field)
    return entities

def add_vector_field(nb, dimension=default_dim):     # line 527 — wins
    field_name = gen_unique_str()
    field = {...}          # built, then discarded
    return field_name      # returns only the name
```

Removed the second — both the broken one and the one that was winning.
Grepped the whole repo: **neither definition has a caller**; every
`add_vector_field` hit is something else
(`test_milvus_client_add_vector_field` test methods, the
`Op.add_vector_field` enum member in `chaos/checker.py`, a `Checker`
method of the same name). Dead-code removal, not an API change.

**Net effect on what executes:** `test_delete_vector_default` ×4
(**L0**) and `test_compact_after_delete_partial` ×2 (L3) start running;
nothing stops running; the 3 negative cases keep running under the new
class name. Computed by parsing each file before and after and resolving
shadowing the way Python does, not by reading the diff.

F811 is now clean across `tests/` with no per-file suppressions.

### Star imports expanded, F403/F405/F821 re-enabled

33 of 307 files used `from utils.util_pymilvus import *` and friends.
Inside those, pyflakes cannot tell a typo from a star-imported name — it
downgrades everything unresolved to F405, and **F821 never fires**. The
`NameError` fixed in commit 3 was only catchable because that file
happens not to use a star import.

The 379 F405 hits were only **58 distinct names**. All are now explicit,
resolved by walking the star modules in source order (later import wins,
matching Python). Names that `util_pymilvus` merely re-exports come from
their real source now — `DataType` from `pymilvus`, `np` from `numpy`,
`log` from `utils.util_log` — rather than being laundered through it.

Turning F821 on immediately caught **two real undefined names**, both in
`rate_limit/test_rate_limit.py` (L3, so it has never run in CI):

```python
for collection_num in range(collectionRateLimit):   # no such name
#                          ^ the parametrize arg is `collection_rate_limit`, and it is
#                            a string ("10"/"500"), so range() needs int().
#                            The same test uses the correct name 30 lines below.

mil.uninstall(release_name, namespace=NAMESPACE)    # no such name
#  ^ MilvusOperator.uninstall has exactly this signature, and other suites do the
#    same under `mic`/`milvus_op` — the instantiation was just missing.
```

Both fixed. `test_milvus_client_data_integrity_new.py` needed 15 `noqa`
markers instead: its `prepare_data` fixture injects those names via
`globals()[field] = ...`, which no static analysis can follow. That
block has other problems — a `float is None` check against the builtin,
an always-true `if "is null" or "IS NULL" in expression`, leftover
`binbin_debug1` logging — but rewriting fixture logic does not belong
here; noted in #52116.

### 13 assertion gaps closed

Of ~80 F841 hits, most are harmless scratch values. 13 were a call whose
result was bound and then never checked. Highlights:

- `test_milvus_client_database.py`: two tests apply a non-existent
property key (`data.replica.number`, note `data.` not `database.`),
describe the database, and drop it without checking anything. Their
docstrings say `actual: run successfully` / `nothing changed` — the
intent was to prove nothing changed. Now asserted.
- `test_milvus_client_alias.py`: two assertions were commented out,
**and they were commented out for a reason** — `list_aliases()[0]`
returns a dict, so `assert alias in aliases` would have tested dict
keys. Rewritten against `[0]["aliases"]` (how the rest of the file does
it) and re-enabled. In the skipped `alias_default` test the calls were
also missing the required `collection_name` argument.

The remaining ~65 F841 stay, and the rule stays off.

### E402, E712, E722 also enabled

**E722** (37 sites): mechanical `except:` → `except Exception:`. Checked
the two ways this could change behaviour, both empty: no bare-except
try-block can raise `SystemExit`/`KeyboardInterrupt`, and **none calls
`pytest.skip`/`fail`/`xfail`**. That second one matters — pytest's
`Skipped` derives from `BaseException`, so a bare except silently eats
it while `except Exception` would not. 0 of 37, so behaviour is
unchanged.

**E402** (5 sites): imports that were just in the wrong place — `import
re` sat 70 lines into `check/param_check.py`, and the two
`resource_group` modules imported `rich` after their module-level
constants. Nothing to do with `sys.path`, which is what an earlier
version of this description wrongly claimed.

**E712** (5 sites), gone through one at a time rather than
bulk-rewritten:

| site | value | change |
|---|---|---|
| `test_milvus_client_partition.py` | `has_partition()` | → `is False` —
stricter than `not result`, which would pass on `None` |
| `test_restart.py` | JSON status flag | → `is True` |
| `test_restart.py` | local `break_flag` | → plain truth test, it is a
real bool |
| `test_query.py` | wrapper `check_result` | → `is True` |
| `test_milvus_client_ttl.py` | `hit.get(bool_field)` | **kept as `==
False` + noqa** |

That last one is the reason this rule cannot be bulk-fixed: `.get()`
returns `None` for a missing field, so `not ...` would pass exactly
where the assertion should fail, and `is False` would break on a numpy
bool.

### What ruff.toml still disables

| rule | n | reason |
|---|---|---|
| F841 | ~65 | tests deliberately keep call results; the 13 that were
genuine assertion gaps are fixed above |
| UP031 | 95 | pure style rewrite that catches no bugs, and 27 sites use
`% (...)` tuples where it is not equivalent |

Everything else pyflakes/pycodestyle checks is on, including `F401`,
`F811` and `F821`, so new code cannot introduce dead, duplicate or
undefined names. Tracked in #52116.

### Verification

Not just "ruff is green" — the mechanical passes were checked for
semantic drift:

- **Format pass:** parsed all 307 files before and after and compared
ASTs with string constants normalized away → **zero structural
differences**. The only AST-visible deltas are 146 string-constant
changes, every one whitespace-only (docstring indentation / trailing
whitespace); zero non-whitespace string changes.
- **Import removals:** 142 imports were dropped. For each, checked
whether the bound name still appears anywhere in its file. 115 have no
remaining reference at all. The 27 that still matched textually were
each read by hand and are all comments (`# pdb.set_trace()`, the
commented-out checker dict in `test_load_with_checker.py`), docstring
prose (`"input random collection name"`, `"Tuple[List[Dict]]"`), string
literals (`"Collection ID"`, the `"config"` dict key), a different
binding (`from time import sleep`), or attribute access through another
module (`ct.CaseLabel`, `np.random`). **No real reference was removed.**
- **Python version:** the pyupgrade rewrites emit `datetime.UTC` (3.11+)
and `X | None` (3.10+). `tests/.python-version` is 3.12,
`tests/scripts/python-env.sh` defaults to 3.12, and
`tests/docker/Dockerfile` is `python:3.12-bullseye`.
- **`compileall`** over all 307 files under Python 3.12 passes.
- No external reference to `TestDeleteVector`,
`test_compact_after_delete`, or `add_vector_field` exists anywhere in
the repo (`*.py`, `*.sh`, `*.yaml`, `*.json`).

### Conflict surface

Of the 100 currently-open PRs, 9 touch `tests/*.py` and only **2**
overlap with this change: #51903 (3 of its 5 files) and #51856 (1 file).
The other 7 touch files that were already format-clean. Worth merging
promptly to keep that number small.

`.pre-commit-config.yaml` needs no change — it already runs with
`--config=tests/ruff.toml`, so local hooks and CI stay in sync.

---------

Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-03 14:54:18 +08:00

492 lines
17 KiB
Python

import re
import sys
from common import common_type as ct
sys.path.append("..")
from collections.abc import Iterable
import numpy as np
from deepdiff import DeepDiff
from utils.util_log import test_log as log
epsilon = ct.epsilon
def deep_approx_compare(x, y, epsilon=epsilon):
"""
Recursively compares two objects for approximate equality, handling floating-point precision.
Args:
x: First object to compare
y: Second object to compare
epsilon: Tolerance for floating-point comparisons (default: 1e-6)
Returns:
bool: True if objects are approximately equal, False otherwise
Handles:
- Numeric types (int, float, numpy scalars)
- Sequences (list, tuple, numpy arrays)
- Dictionaries
- Other iterables (except strings)
- Numpy arrays (shape and value comparison)
- Falls back to strict equality for other types
"""
# Handle basic numeric types (including numpy scalars)
if isinstance(x, (int, float, np.integer, np.floating)) and isinstance(y, (int, float, np.integer, np.floating)):
return abs(float(x) - float(y)) < epsilon
# Handle lists/tuples/arrays
if isinstance(x, (list, tuple, np.ndarray)) and isinstance(y, (list, tuple, np.ndarray)):
if len(x) != len(y):
return False
for a, b in zip(x, y):
if not deep_approx_compare(a, b, epsilon):
return False
return True
# Handle dictionaries
if isinstance(x, dict) and isinstance(y, dict):
if set(x.keys()) != set(y.keys()):
return False
for key in x:
if not deep_approx_compare(x[key], y[key], epsilon):
return False
return True
# Handle other iterables (e.g., Protobuf containers)
if isinstance(x, Iterable) and isinstance(y, Iterable) and not isinstance(x, str):
try:
return deep_approx_compare(list(x), list(y), epsilon)
except Exception:
pass
# Handle numpy arrays
if isinstance(x, np.ndarray) and isinstance(y, np.ndarray):
if x.shape != y.shape:
return False
return np.allclose(x, y, atol=epsilon)
# Fall back to strict equality for other types
return x == y
# Pre-compile regex patterns for better performance
_GEO_PATTERN = re.compile(r"(POINT|LINESTRING|POLYGON)\s+\(")
_WHITESPACE_PATTERN = re.compile(r"\s+")
def normalize_geo_string(s):
"""
Normalize a GEO string by removing extra whitespace.
Args:
s: String value that might be a GEO type (POINT, LINESTRING, POLYGON)
Returns:
Normalized GEO string or original value if not a GEO string
"""
if isinstance(s, str) and s.startswith(("POINT", "LINESTRING", "POLYGON")):
s = _GEO_PATTERN.sub(r"\1(", s)
s = _WHITESPACE_PATTERN.sub(" ", s).strip()
return s
def normalize_value(value):
"""
Normalize values for comparison by converting to standard types and formats.
"""
# Fast path for None and simple immutable types
if value is None or isinstance(value, (bool, int)):
return value
# Convert numpy types to Python native types
if isinstance(value, (np.integer, np.floating)):
return float(value) if isinstance(value, np.floating) else int(value)
# Handle strings (common case for GEO fields)
if isinstance(value, str):
return normalize_geo_string(value)
# Convert list-like protobuf/custom types to standard list
type_name = type(value).__name__
if type_name in ("RepeatedScalarContainer", "HybridExtraList", "RepeatedCompositeContainer"):
value = list(value)
# Handle list of dicts (main use case for search/query results)
if isinstance(value, (list, tuple)):
normalized_list = []
for item in value:
if isinstance(item, dict):
# Normalize GEO strings in dict values
normalized_dict = {}
for k, v in item.items():
if isinstance(v, str):
normalized_dict[k] = normalize_geo_string(v)
elif isinstance(v, (np.integer, np.floating)):
normalized_dict[k] = float(v) if isinstance(v, np.floating) else int(v)
elif isinstance(v, np.ndarray):
normalized_dict[k] = v.tolist()
elif type(v).__name__ in (
"RepeatedScalarContainer",
"HybridExtraList",
"RepeatedCompositeContainer",
):
normalized_dict[k] = list(v)
else:
normalized_dict[k] = v
normalized_list.append(normalized_dict)
else:
# For non-dict items, just add as-is
normalized_list.append(item)
return normalized_list
# Return as-is for other types
return value
def compare_lists_with_epsilon_ignore_dict_order(a, b, epsilon=epsilon):
"""
Compares two lists of dictionaries for equality (order-insensitive) with floating-point tolerance.
Args:
a (list): First list of dictionaries to compare
b (list): Second list of dictionaries to compare
epsilon (float, optional): Tolerance for floating-point comparisons. Defaults to 1e-6.
Returns:
bool: True if lists contain equivalent dictionaries (order doesn't matter), False otherwise
Note:
Uses deep_approx_compare() for dictionary comparison with floating-point tolerance.
Maintains O(n²) complexity due to nested comparisons.
"""
if len(a) != len(b):
return False
a = normalize_value(a)
b = normalize_value(b)
# Create a set of available indices for b
available_indices = set(range(len(b)))
for item_a in a:
matched = False
# Create a list of indices to remove (avoid modifying the set during iteration)
to_remove = []
for idx in available_indices:
if deep_approx_compare(item_a, b[idx], epsilon):
to_remove.append(idx)
matched = True
break
if not matched:
return False
# Remove matched indices
available_indices -= set(to_remove)
return True
def compare_lists_with_epsilon_ignore_dict_order_deepdiff(a, b, epsilon=epsilon):
"""
Compare two lists of dictionaries for equality (order-insensitive) with floating-point tolerance using DeepDiff.
"""
# Normalize both lists to handle type differences
a_normalized = normalize_value(a)
b_normalized = normalize_value(b)
# Check length first
if len(a_normalized) != len(b_normalized):
log.debug(
f"[COMPARE_LISTS] Length mismatch: Query result length({len(a_normalized)}) != Expected result length({len(b_normalized)})"
)
return False
for i in range(len(a_normalized)):
diff = DeepDiff(
a_normalized[i],
b_normalized[i],
ignore_order=True,
math_epsilon=epsilon,
significant_digits=1,
ignore_type_in_groups=[(list, tuple)],
ignore_string_type_changes=True,
)
if diff:
log.debug(f"[COMPARE_LISTS] Found differences at row {i}: {diff}")
return False
return True
def ip_check(ip):
if ip == "localhost":
return True
if not isinstance(ip, str):
log.error("[IP_CHECK] IP(%s) is not a string." % ip)
return False
return True
def number_check(num):
if str(num).isdigit():
return True
else:
log.error("[NUMBER_CHECK] Number(%s) is not a numbers." % num)
return False
def exist_check(param, _list):
if param in _list:
return True
else:
log.error("[EXIST_CHECK] Param(%s) is not in (%s)." % (param, _list))
return False
def dict_equal_check(dict1, dict2):
"""Check if dict2 is a subset of dict1.
This allows API responses to include additional fields without breaking tests.
For example, if dict1 = {'a': 1, 'b': 2, 'c': 3} and dict2 = {'a': 1, 'b': 2},
the check will pass because all key-value pairs in dict2 exist in dict1.
"""
if not isinstance(dict1, dict) or not isinstance(dict2, dict):
log.error("[DICT_EQUAL_CHECK] Type of dict(%s) or dict(%s) is not a dict." % (str(dict1), str(dict2)))
return False
# Check if dict2 is a subset of dict1
return all(k in dict1 and dict1[k] == v for k, v in dict2.items())
def list_de_duplication(_list):
if not isinstance(_list, list):
log.error("[LIST_DE_DUPLICATION] Type of list(%s) is not a list." % str(_list))
return _list
# de-duplication of _list
result = list(set(_list))
# Keep the order of the elements unchanged
result.sort(key=_list.index)
log.debug(
"[LIST_DE_DUPLICATION] %s after removing the duplicate elements, the list becomes %s"
% (str(_list), str(result))
)
return result
def list_equal_check(param1, param2):
check_result = True
if len(param1) == len(param1):
_list1 = list_de_duplication(param1)
_list2 = list_de_duplication(param2)
if len(_list1) == len(_list2):
for i in _list1:
if i not in _list2:
check_result = False
break
else:
check_result = False
else:
check_result = False
if check_result is False:
log.error("[LIST_EQUAL_CHECK] List(%s) and list(%s) are not equal." % (str(param1), str(param2)))
return check_result
def list_contain_check(sublist, superlist):
if not isinstance(sublist, list):
raise Exception("%s isn't list type" % sublist)
if not isinstance(superlist, list):
raise Exception("%s isn't list type" % superlist)
check_result = True
for i in sublist:
if i not in superlist:
check_result = False
break
else:
superlist.remove(i)
if not check_result:
# truncate the lists to 100 items in log message
log.error(f"list_contain_check: List({str(superlist[:20])}...) does not contain list({str(sublist[:20])}...)")
return check_result
def get_connect_object_name(_list):
"""get the name of the objects that returned by the connection"""
if not isinstance(_list, list):
log.error("[GET_CONNECT_OBJECT_NAME] Type of list(%s) is not a list." % str(_list))
return _list
new_list = []
for i in _list:
if not isinstance(i, tuple):
log.error(
"[GET_CONNECT_OBJECT_NAME] The element:%s of the list is not tuple, please check manually." % str(i)
)
return _list
if len(i) != 2:
log.error(
"[GET_CONNECT_OBJECT_NAME] The length of the tuple:%s is not equal to 2, please check manually."
% str(i)
)
return _list
if i[1] is not None:
_obj_name = type(i[1]).__name__
new_list.append((i[0], _obj_name))
else:
new_list.append(i)
log.debug("[GET_CONNECT_OBJECT_NAME] list:%s is reset to list:%s" % (str(_list), str(new_list)))
return new_list
def equal_entity(exp, actual):
"""
compare two entities containing vector field
{"int64": 0, "float": 0.0, "float_vec": [0.09111554112502457, ..., 0.08652634258062468]}
:param exp: exp entity
:param actual: actual entity
:return: bool
"""
assert actual.keys() == exp.keys()
for field, value in exp.items():
if isinstance(value, list):
assert len(actual[field]) == len(exp[field])
for i in range(0, len(exp[field]), 4):
assert abs(actual[field][i] - exp[field][i]) < ct.epsilon
else:
assert actual[field] == exp[field]
return True
def entity_in(entity, entities, primary_field):
"""
according to the primary key to judge entity in the entities list
:param entity: dict
{"int": 0, "vec": [0.999999, 0.111111]}
:param entities: list of dict
[{"int": 0, "vec": [0.999999, 0.111111]}, {"int": 1, "vec": [0.888888, 0.222222]}]
:param primary_field: collection primary field
:return: True or False
"""
primary_default = ct.default_primary_field_name
primary_field = primary_default if primary_field is None else primary_field
primary_key = entity.get(primary_field, None)
primary_keys = []
for e in entities:
primary_keys.append(e[primary_field])
if primary_key not in primary_keys:
return False
index = primary_keys.index(primary_key)
return equal_entity(entities[index], entity)
def remove_entity(entity, entities, primary_field):
"""
according to the primary key to remove an entity from an entities list
:param entity: dict
{"int": 0, "vec": [0.999999, 0.111111]}
:param entities: list of dict
[{"int": 0, "vec": [0.999999, 0.111111]}, {"int": 1, "vec": [0.888888, 0.222222]}]
:param primary_field: collection primary field
:return: entities of removed entity
"""
primary_default = ct.default_primary_field_name
primary_field = primary_default if primary_field is None else primary_field
primary_key = entity.get(primary_field, None)
primary_keys = []
for e in entities:
primary_keys.append(e[primary_field])
index = primary_keys.index(primary_key)
entities.pop(index)
return entities
def equal_entities_list(exp, actual, primary_field, with_vec=False):
"""
compare two entities lists in inconsistent order
:param with_vec: whether entities with vec field
:param exp: exp entities list, list of dict
:param actual: actual entities list, list of dict
:return: True or False
example:
exp = [{"int": 0, "vec": [0.999999, 0.111111]}, {"int": 1, "vec": [0.888888, 0.222222]}]
actual = [{"int": 1, "vec": [0.888888, 0.222222]}, {"int": 0, "vec": [0.999999, 0.111111]}]
exp = actual
"""
exp = exp.copy()
if len(exp) != len(actual):
return False
if with_vec:
for a in actual:
# if vec field returned in query res
if entity_in(a, exp, primary_field):
try:
# if vec field returned in query res
remove_entity(a, exp, primary_field)
except Exception as ex:
log.error(ex)
else:
for a in actual:
if a in exp:
try:
exp.remove(a)
except Exception as ex:
log.error(ex)
return True if len(exp) == 0 else False
def output_field_value_check(search_res, original, pk_name):
"""
check if the value of output fields is correct, it only works on auto_id = False
:param search_res: the search result of specific output fields
:param original: the data in the collection
:return: True or False
"""
pk_name = ct.default_primary_field_name if pk_name is None else pk_name
nq = len(search_res)
limit = len(search_res[0])
check_nqs = min(2, nq) # the output field values are wrong only at nq>=2 #45338
for n in range(check_nqs):
for i in range(limit):
entity = search_res[n][i].fields
_id = search_res[n][i].id
for field in entity.keys():
if isinstance(entity[field], list):
for order in range(0, len(entity[field]), 4):
assert abs(original[field][_id][order] - entity[field][order]) < ct.epsilon
elif isinstance(entity[field], dict) and field != ct.default_json_field_name:
# sparse vector checking: compare keys (indices) of the sparse vector
num = original[original[pk_name] == _id].index.to_list()[0]
assert entity[field].keys() == original[field][num].keys()
elif isinstance(entity[field], bytes):
# bfloat16/float16/binary vectors returned as bytes — skip value check
# (numpy dtype 'E' cannot be compared via ufunc 'equal' or converted to buffer)
continue
else:
num = original[original[pk_name] == _id].index.to_list()[0]
expected_val = original[field][num]
# pandas converts None to NaN, while Milvus returns None for nullable fields
if entity[field] is None and (
expected_val is None or (isinstance(expected_val, float) and np.isnan(expected_val))
):
continue
assert expected_val == entity[field], f"the output field values are wrong at nq={n}"
return True