test: run Weaviate serde and conversion tests in the unit job - #3955
Open
dudanogueira wants to merge 4 commits into
Open
dudanogueira wants to merge 4 commits into
dudanogueira wants to merge 4 commits into
Conversation
dudanogueira
requested review from
julian-risch
and removed request for
a team
September 14, 2026 17:55
|
|
4 tasks done
Contributor
|
Hi @dudanogueira, thanks for your interest in contributing to Haystack! 🙏 This is an automated message to help us keep the review queue healthy. |
github-actions
Bot
added
the
type:documentation
Contributor
Coverage report (weaviate)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Weaviate tracks client integration usage via the X-Weaviate-Client-Integration header (weaviate/weaviate#10535). Tag the connection with "haystack-python/<version>" so weaviate-haystack adoption shows up in Weaviate telemetry. The header is merged into `additional_headers` at client construction, so it reaches both the HTTP and the gRPC transport, on all four connection paths (sync/async x Weaviate Cloud/custom). The alternative, `client.integrations.configure()`, relies on a private `_IntegrationConfig` and on the Weaviate Cloud path runs after `connect_to_weaviate_cloud()` has already built the httpx client, which would drop the HTTP header. A user supplied value for the same header always wins, so it can be overridden or suppressed via `additional_headers`. The merged headers are not stored on the instance, so `to_dict()` keeps serializing only what the user passed and a serialized pipeline does not bake in the version of weaviate-haystack that wrote it. The version is read at runtime via importlib.metadata with an "unknown" fallback, mirroring the existing `_attribution_header()` helper in the perplexity integration.
`TestWeaviateDocumentStore` carries a class-level `@pytest.mark.integration`, so every test inside it was excluded from `hatch run test:unit`. Seven of them need no running Weaviate: they either patch the client or exercise pure functions. As a result the unit coverage number posted on every PR, and the badge in the repo README, never accounted for `to_dict()`, `from_dict()`, `_to_data_object()` or `_to_document()`, and a regression in any of them could only be caught by the integration job. Move those seven into a new unmarked `TestWeaviateDocumentStoreSerde` class: test_connection, test_to_dict, test_to_dict_and_from_dict_preserves_grpc_settings, test_from_dict, test_to_data_object, test_to_document and test_schema_class_name_conversion_preserves_pascal_case. No test body changes; this is a pure relocation. Unit selection goes from 87 to 94 and integration from 187 to 180, with the total unchanged at 274.
dudanogueira
force-pushed
the
test/weaviate-unit-job-coverage
branch
from
September 15, 2026 18:35
47de725 to
a4ad429
Compare
4 tasks done
Merging origin/main into this branch (0f1247e) left the block of X-Weaviate-Client-Integration header tests in test_document_store.py twice, byte-for-byte identical. Python silently lets the second definition shadow the first, so the tests still ran, but ruff flags each redefinition as F811 and the lint step failed CI with 5 errors: - test_client_sends_integration_header - test_async_client_sends_integration_header - test_user_supplied_integration_header_wins - test_integration_header_value_falls_back_to_unknown_version - test_integration_header_is_not_serialized Remove the second copy. No test coverage changes: each test is still defined once, lint passes and the unit suite passes (94 tests). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Proposed Changes:
Stacked on #3954 — the diff here is the second commit only; please merge that one first.
TestWeaviateDocumentStorecarries a class-level@pytest.mark.integration, so every test inside it is excluded fromhatch run test:unit. Seven of them need no running Weaviate: they either patch the client or exercise pure functions.The consequence is that the unit coverage number posted on every PR — and the
weaviatebadge in the repo README — never accounted forto_dict(),from_dict(),_to_data_object()or_to_document(). A regression in any of them could only be caught by the integration job.This moves those seven into a new unmarked
TestWeaviateDocumentStoreSerdeclass:test_connectiontest_to_dicttest_to_dict_and_from_dict_preserves_grpc_settingstest_from_dicttest_to_data_objecttest_to_documenttest_schema_class_name_conversion_preserves_pascal_caseThe marker itself is load-bearing and stays: the
document_storefixture talks to a live server and the class inherits fivehaystack.testingsuites. Extracting the seven is a much smaller and safer change than removing the class marker and re-marking the remaining ~40 methods individually.How did you test it?
No test bodies change; this is a pure relocation. The counts confirm it: unit selection goes from 87 to 94 and integration from 187 to 180, with the total unchanged at 274.
hatch run test:unit(94 passed),hatch run test:integration(179 passed, 1 skipped),hatch run fmt-checkandhatch run test:typesall pass.Notes for the reviewer
git diffrenders this as a large shuffle because the new class is inserted above the existing one.git diff --color-movedor reviewing with whitespace-insensitive move detection makes it much easier to read.Checklist