Skip to content

feat(connector-tests): mock-server tests — spec, harness, jira reference suite, coverage gate - #1723

Merged
mitasovr merged 10 commits into
constructorfabric:mainfrom
mitasovr:feat/connector-mock-tests-spec
Jul 13, 2026
Merged

feat(connector-tests): mock-server tests — spec, harness, jira reference suite, coverage gate#1723
mitasovr merged 10 commits into
constructorfabric:mainfrom
mitasovr:feat/connector-mock-tests-spec

Conversation

@mitasovr

@mitasovr mitasovr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduces connector mock-server tests (L1 of the connector test ladder): the normative spec, the shared pytest harness, a reference suite for the jira connector, and CI coverage-gate wiring. Credential-free, Docker-free, network-free — the only deterministic CI check of connector behavior (pagination, cursors, error handling, transformations).

Spec — docs/domain/connector/specs/feature-connector-mock-tests/FEATURE.md

Test ladder L0 static validation → L1 mock tests (this PR) → L2 live smoke → L3 pipeline e2e. A mock test loads connector.yaml in-process through the pinned airbyte-cdk (same entry point as the source-declarative-manifest image), intercepts HTTP at the transport layer (HttpMocker — an unmatched request fails the test), and runs a full protocol read. Pattern adapted from certified upstream connectors (source-jira unit_tests/mock_server/); CAT deliberately not adopted (needs per-connector images + live sandbox creds — our L2 covers that).

Harness — src/ingestion/tests/connectors (package connector_tests)

  • get_source (manifest → in-process source, config validated against the manifest spec), read_stream (protocol read via entrypoint_wrapper)
  • ConfigBuilder base always carrying insight_tenant_id / insight_source_id
  • assert_records_conform: schema from schemas/<stream>.json or the manifest inline schema; strict mode flags record keys the schema does not declare (manifest↔schema drift)
  • http_mocker pytest fixture: on a passing test every registered matcher must have been hit
  • a bare pytest run collects the harness's own tests + every nocode connector suite; CDK connectors are excluded (own pins, own coverage components)
  • airbyte-cdk pinned to the 6.60.x line matching the manifests' version: header

Reference suite — src/ingestion/connectors/task-tracking/jira/tests

  • jira_projects: single page, OffsetIncrement pagination with exact startAt matchers, empty page, tenant/source/unique_key stamping, strict schema conformance, 429 retry, 400 ignore
  • jira_issue_keys: substream partition per project, nextPageToken pagination, cursor hoist from fields.updated, state emission + resume read asserting the JQL filter (cursor − PT14H lookback) via an exact request matcher

Found by the rig (kept as an explicit skip with reason): jira_issue_keys.jira_id is declared ['string','null'] but CDK Jinja literal-eval emits int for numeric ids (the sibling jira_projects.project_id, generated from real data, is correctly number). Fixing the schema implies a bronze column-type change — tracked separately.

Coverage — wired like every other component

connector-mock-tests registered in scripts/ci/components.py (lang: python, cov_package: connector_tests; paths include the harness + the jira package, longest-prefix keeps jira-enrich separate). The standard python CI job produces the Cobertura report; the shared gate applies the usual ≥80% overall / ≥80% new-code thresholds. Line coverage measures the harness (declarative YAML has no first-party lines); a connector's behavioral coverage is the spec's stream coverage matrix, with mandatory explicit skip reasons.

Skill wiring — .claude/skills/connector/

  • create.md §5.7 (mandatory before deploy): authoring step with layout, fixture rules, coverage matrix, fixture-based skeleton, literal-eval / exact-matcher gotchas
  • test.md Phase 3: run mock tests after manifest validation, before any live credential testing; missing tests/ reported as a spec gap
  • validate.md: pytest in Step 1 + tests/ in the structure checklist

Test plan

  • Local: pytest in the harness root — 17 passed, 1 skipped (the documented drift), no credentials, no Docker
  • Local run of the real gate (scripts/ci/coverage.py gate): connector-mock-tests 95.0% ≥ 80% PASS
  • Standalone per-suite invocation (pytest src/ingestion/connectors/task-tracking/jira/tests/) verified
  • cfs validate + validate-toc green on the spec

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added credential-free mock-server testing for declarative connectors, covering records, schemas, pagination, retries, incremental state, and request handling.
    • Added a reference test suite for Jira connector streams.
    • Added CI coverage and gating for connector mock-test suites.
  • Documentation

    • Updated connector creation, testing, validation, and reference documentation with required test phases, coverage expectations, and reporting guidance.
  • Bug Fixes

    • Improved detection and reporting when required connector test suites are missing or fail.

…the connector skill

Add feature-connector-mock-tests spec (L1 of the connector test ladder):
credential-free pytest suites that load connector.yaml in-process through
the pinned airbyte-cdk, intercept HTTP with HttpMocker, and run a full
protocol read — covering pagination, incremental cursors, error handling,
transformations (tenant_id/source_id/unique_key stamping), substream
partitioning, and schema conformance. Adapted from the certified upstream
Airbyte pattern (source-jira mock_server suites); CAT is deliberately not
adopted (needs per-connector Docker images + live sandbox creds).

Wire the step into the connector skill:
- create.md §5.7: mandatory mock-test authoring before deploy (layout,
  fixture rules, coverage matrix, skeleton)
- test.md Phase 3: run mock tests after manifest validation, before any
  live credential testing
- validate.md: pytest run in Step 1 + tests/ presence in the structure
  checklist
- SKILL.md: spec added to References, test command description updated

Register the FEATURE in .cf-studio/config/artifacts.toml (DOCS-ONLY) and
index it in the connector domain README. cfs validate + validate-toc green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
@mitasovr
mitasovr requested a review from a team as a code owner July 9, 2026 13:53
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mitasovr, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bb0e1d87-6cdc-4a0c-8436-bf90098ad5d3

📥 Commits

Reviewing files that changed from the base of the PR and between ce51471 and e49d53d.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • scripts/ci/changed.py
  • scripts/ci/components.py
📝 Walkthrough

Walkthrough

This PR defines credential-free connector mock-server testing, adds a reusable pytest harness, implements Jira reference coverage, integrates mock tests into connector workflows, and registers CI coverage ownership.

Changes

Connector Mock-Server Testing

Layer / File(s) Summary
Specification and workflow requirements
docs/domain/connector/specs/..., .claude/skills/connector/..., .cf-studio/config/artifacts.toml, docs/domain/connector/README.md
Defines mock-test behavior, coverage requirements, workflow phases, validation rules, and documentation registration.
Shared pytest harness
src/ingestion/tests/connectors/...
Adds connector discovery, strict HTTP mocking, in-process CDK reads, config builders, fixture loading, schema assertions, package exports, and pytest configuration.
Jira reference connector tests
src/ingestion/connectors/task-tracking/jira/tests/*
Adds mocked Jira tests for pagination, retries, empty responses, schema behavior, stamping, substream partitioning, and incremental resume filtering.
Harness validation and CI ownership
src/ingestion/tests/connectors/meta/*, src/ingestion/tests/connectors/README.md, scripts/ci/*, .github/workflows/ci.yml
Tests harness contracts, documents execution conventions, and adds CI matrix arguments, co-triggering, and component ownership.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Pytest
  participant HttpMocker
  participant ConnectorHarness
  participant JiraConnector
  Pytest->>HttpMocker: Register Jira response fixtures
  Pytest->>ConnectorHarness: Execute read_stream
  ConnectorHarness->>JiraConnector: Load connector.yaml and run protocol read
  JiraConnector->>HttpMocker: Request mocked Jira pages
  HttpMocker-->>JiraConnector: Return fixture responses
  ConnectorHarness-->>Pytest: Return records and state
  Pytest->>ConnectorHarness: Validate schema and behavior
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers: mozhaev-dev

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: connector mock-server tests, supporting spec and harness work, Jira reference suite, and CI coverage gating.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mitasovr mitasovr linked an issue Jul 9, 2026 that may be closed by this pull request

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/connector/workflows/create.md:
- Around line 823-842: The fixture-loading example is incomplete because
`load_fixture()` is referenced in `test_users_full_refresh` without being
defined or imported by the shared harness contract. Update the example to use
the documented fixture-loading helper from the connector test harness, or inline
the fixture-loading logic directly in the snippet so authors can run it as-is.
Keep the rest of the `freezegun`, `HttpMocker`, and `read_stream` example
unchanged.

In @.claude/skills/connector/workflows/test.md:
- Around line 32-43: The mock-test fallback in the workflow does not explicitly
stop when the connector has no tests directory, which can let execution continue
into credentialed phases incorrectly. Update the Phase 3 instructions in the
connector workflow doc so the `tests/`-missing branch in the mock-server test
flow is an explicit STOP/fail state before Phase 4, alongside the existing
`pytest` and manifest-fix guidance.

In @.claude/skills/connector/workflows/validate.md:
- Around line 24-31: The missing-suite branch in validate.md is not a real
validation gate, and the stale connector-create.md reference should be updated
to create.md. In the workflow section that runs the mock-server suite and checks
for tests/, make the “report as a spec gap” path explicitly stop or fail the
validation so connectors without tests/ cannot proceed as compliant. Keep the
gating behavior tied to the validation step and the existing mock-test suite
flow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c6c43e3-f2db-4eec-a068-77d1dfe976d7

📥 Commits

Reviewing files that changed from the base of the PR and between d681dce and 6cf251e.

📒 Files selected for processing (7)
  • .cf-studio/config/artifacts.toml
  • .claude/skills/connector/SKILL.md
  • .claude/skills/connector/workflows/create.md
  • .claude/skills/connector/workflows/test.md
  • .claude/skills/connector/workflows/validate.md
  • docs/domain/connector/README.md
  • docs/domain/connector/specs/feature-connector-mock-tests/FEATURE.md

Comment thread .claude/skills/connector/workflows/create.md
Comment thread .claude/skills/connector/workflows/test.md
Comment thread .claude/skills/connector/workflows/validate.md
@ktursunov

ktursunov commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@mitasovr please find a way to track tested connectors in ci - to see report what's covered vs what's missing

@mitasovr

mitasovr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@mitasovr please find a way to track tested connectors in ci - to see report what's covered vs what's missing

yes
i'll try
but in next PR with tests

@mitasovr
mitasovr enabled auto-merge (squash) July 10, 2026 07:48
Roman Mitasov and others added 2 commits July 10, 2026 15:29
…e + coverage gate

Implement L1 of the connector test ladder per the spec in this PR:

Harness (src/ingestion/tests/connectors, installable package connector_tests):
- get_source: in-process YamlDeclarativeSource from a package's connector.yaml
  (same CDK entry point as the source-declarative-manifest image), config
  validated against the manifest spec first; airbyte-cdk pinned to the 6.60.x
  line matching the manifests' version header
- read_stream: full protocol read via airbyte_cdk.test.entrypoint_wrapper
- ConfigBuilder base always carrying insight_tenant_id / insight_source_id
- assert_records_conform: schema from schemas/<stream>.json or the manifest
  InlineSchemaLoader; strict mode flags record keys the schema does not
  declare (manifest<->schema drift)
- http_mocker pytest fixture (transport-level, no network fallthrough; on a
  passing test every registered matcher must have been hit)
- harness_plugin: a bare pytest run collects meta/ plus every NOCODE
  connector's tests/ (connector.yaml present, no pyproject.toml); CDK
  connectors keep their own suites, pins, and coverage components

Jira reference suite (task-tracking/jira/tests):
- jira_projects: single page, OffsetIncrement pagination with exact
  startAt matchers, empty page, tenant/source/unique_key stamping, strict
  schema conformance, 429 retry, 400 ignore
- jira_issue_keys: substream partition per project, nextPageToken pagination,
  cursor hoist from fields.updated, state emission + resume read asserting the
  jql filter (cursor minus PT14H lookback) via an exact request matcher
- schema_conformance explicitly skipped with reason: the rig found a real
  drift — jira_id declared ['string','null'] but Jinja literal-eval emits int
  for numeric ids (cf. jira_projects.project_id: number); schema fix implies a
  bronze column-type change, tracked separately

Coverage, wired like every other component: connector-mock-tests registered in
scripts/ci/components.py (lang python, cov_package connector_tests, paths
include the harness and the jira package; longest-prefix keeps jira-enrich
separate). Local run of the real gate: 95.0% >= 80% PASS. Line coverage
measures the harness — manifests have no first-party lines; behavioral
coverage of a connector is the spec's stream matrix.

Also: spec updated (reference implementation youtrack->jira, CI/coverage DoD,
harness paths) and the connector skill's 5.7 skeleton aligned with the real
fixture-based API. 17 passed, 1 skipped locally.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/ingestion/tests/connectors/pyproject.toml (1)

20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding an explicit upper bound to the jsonschema constraint.

The comment states CDK pins jsonschema >=4.17.3,<4.18, but the declared constraint is only >=4.17.3. While pip's resolver should honor the transitive CDK pin, making the upper bound explicit would prevent accidental upgrades if the CDK pin changes and would keep the comment and constraint in sync.

♻️ Tighten the constraint
-    # airbyte-cdk 6.60.x itself pins jsonschema >=4.17.3,<4.18 — stay inside it.
-    "jsonschema>=4.17.3",
+    # airbyte-cdk 6.60.x itself pins jsonschema >=4.17.3,<4.18 — stay inside it.
+    "jsonschema>=4.17.3,<4.18",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/tests/connectors/pyproject.toml` around lines 20 - 21, Update
the jsonschema dependency constraint in the ingestion test project to include
the explicit upper bound stated in the comment: require version 4.17.3 or newer
but below 4.18, keeping the declaration synchronized with the airbyte-cdk
compatibility range.
src/ingestion/tests/connectors/connector_tests/source.py (1)

49-68: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

load_manifest is called twice in get_source.

_validate_config_against_spec(load_manifest(connector_path), config) reads and parses the YAML, then YamlDeclarativeSource reads the same file again from disk. Consider loading the manifest once and passing it to both the validator and the source constructor (if the CDK API supports it), or at least caching the parsed result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/tests/connectors/connector_tests/source.py` around lines 49 -
68, Eliminate the duplicate manifest load in get_source: load connector.yaml
once, reuse the parsed manifest for _validate_config_against_spec, and pass that
parsed result to YamlDeclarativeSource if its CDK API supports it; otherwise add
appropriate caching so the same file is not parsed twice.
src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py (1)

199-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Resume read bypasses http_mocker fixture validation.

The manual HttpMocker() at line 199 is not subject to _validate_all_matchers_called() since it's outside the fixture. If the resume read's request sequence changes (e.g., an extra unregistered request is added by a future manifest change), the stale matcher would silently go unused without detection. Consider adding an explicit resume_mocker._validate_all_matchers_called() before the with block exits.

🔧 Optional fix
         second = read_stream(_CONNECTOR, _STREAM, config, state=state)
 
         assert len(second.records) == 0
         assert not second.errors
+        resume_mocker._validate_all_matchers_called()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py`
around lines 199 - 213, Ensure the manually created resume_mocker validates all
registered matchers by calling resume_mocker._validate_all_matchers_called()
after read_stream completes and before exiting the with resume_mocker block,
preserving detection of unused or unexpected request matchers in the resume
test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ingestion/tests/connectors/connector_tests/plugin.py`:
- Around line 47-58: Update pytest_collectstart to evict the cached config
module from sys.modules whenever switching to a different connector suite, while
preserving the current suite path insertion behavior. Track the previously
active suite directory (using a module-level state variable) and remove
sys.modules["config"] before importing the new suite’s config.py; ensure
repeated collection of the same suite does not unnecessarily evict it.

In `@src/ingestion/tests/connectors/README.md`:
- Line 13: Update the fenced code block in the connector README to specify an
explicit language, preferably `text`, on its opening fence so it satisfies
Markdown rule MD040.

---

Nitpick comments:
In `@src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py`:
- Around line 199-213: Ensure the manually created resume_mocker validates all
registered matchers by calling resume_mocker._validate_all_matchers_called()
after read_stream completes and before exiting the with resume_mocker block,
preserving detection of unused or unexpected request matchers in the resume
test.

In `@src/ingestion/tests/connectors/connector_tests/source.py`:
- Around line 49-68: Eliminate the duplicate manifest load in get_source: load
connector.yaml once, reuse the parsed manifest for
_validate_config_against_spec, and pass that parsed result to
YamlDeclarativeSource if its CDK API supports it; otherwise add appropriate
caching so the same file is not parsed twice.

In `@src/ingestion/tests/connectors/pyproject.toml`:
- Around line 20-21: Update the jsonschema dependency constraint in the
ingestion test project to include the explicit upper bound stated in the
comment: require version 4.17.3 or newer but below 4.18, keeping the declaration
synchronized with the airbyte-cdk compatibility range.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 543b5207-ee94-450b-a7cb-3476b3b90e0e

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf251e and 61899cf.

📒 Files selected for processing (19)
  • .claude/skills/connector/workflows/create.md
  • .claude/skills/connector/workflows/test.md
  • .claude/skills/connector/workflows/validate.md
  • docs/domain/connector/specs/feature-connector-mock-tests/FEATURE.md
  • scripts/ci/components.py
  • src/ingestion/connectors/task-tracking/jira/tests/config.py
  • src/ingestion/connectors/task-tracking/jira/tests/conftest.py
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_projects.py
  • src/ingestion/tests/connectors/README.md
  • src/ingestion/tests/connectors/conftest.py
  • src/ingestion/tests/connectors/connector_tests/__init__.py
  • src/ingestion/tests/connectors/connector_tests/builders.py
  • src/ingestion/tests/connectors/connector_tests/plugin.py
  • src/ingestion/tests/connectors/connector_tests/schema_assert.py
  • src/ingestion/tests/connectors/connector_tests/source.py
  • src/ingestion/tests/connectors/harness_plugin.py
  • src/ingestion/tests/connectors/meta/test_harness.py
  • src/ingestion/tests/connectors/pyproject.toml
✅ Files skipped from review due to trivial changes (2)
  • src/ingestion/tests/connectors/connector_tests/init.py
  • .claude/skills/connector/workflows/validate.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • .claude/skills/connector/workflows/test.md
  • .claude/skills/connector/workflows/create.md
  • docs/domain/connector/specs/feature-connector-mock-tests/FEATURE.md

Comment thread src/ingestion/tests/connectors/connector_tests/plugin.py
Comment thread src/ingestion/tests/connectors/README.md Outdated
@mitasovr mitasovr changed the title docs(connector): mock-server tests spec + test-authoring step in the connector skill feat(connector-tests): mock-server tests — spec, harness, jira reference suite, coverage gate Jul 10, 2026
@mitasovr
mitasovr requested a review from ktursunov July 10, 2026 13:15
@mitasovr mitasovr linked an issue Jul 10, 2026 that may be closed by this pull request
Roman Mitasov and others added 2 commits July 10, 2026 17:08
…ates, connector coverage report

Review fixes (CodeRabbit + ktursunov):

- plugin.py: evict the cached 'config' module when collection moves to another
  suite — every suite names its builder module config.py and sys.modules would
  otherwise serve the FIRST suite's module to the second (latent bug before any
  second nocode suite lands)
- connector coverage report (requested by review): pytest_terminal_summary now
  prints covered-vs-missing for every connector package (descriptor.yaml) —
  nocode suites counted against this harness, CDK marked as own-component —
  and appends a markdown table to GITHUB_STEP_SUMMARY on CI
- test.md Phase 3: missing tests/ is now an explicit STOP (author the suite, or
  the user explicitly opts into legacy live-testing with the gap recorded)
- validate.md: missing tests/ FAILS validation outright; fix the stale
  connector-create.md reference -> create.md
- pyproject: jsonschema upper bound <4.18 made explicit (matches the CDK pin)
- source.py: lru_cache on load_manifest (spec validation + schema asserts
  re-read the manifest many times per suite)
- README: MD040 fence language

19 passed, 1 skipped locally; connector_tests coverage 96%.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
conftest.py # sys.path for local modules + `from connector_tests.plugin import *`
config.py # ConfigBuilder extending the shared base (always carries insight_tenant_id / insight_source_id)
test_<stream>.py # one module per stream
fixtures/*.json # response bodies: shapes from real API, values synthetic (optional — small responses inline)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest to make them mandatory - to have similar approach for big and small tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's unit test to the test harness - worth running in separate job or workflow - to have clean results

def pytest_load_initial_conftests(early_config, parser, args):
if getattr(early_config.known_args_namespace, "file_or_dir", None):
return # explicit paths given — collect only those
injected = [str(_HARNESS_ROOT / "meta")] + [str(p) for p in nocode_suite_dirs()]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How nocode connectors are tested?

Roman Mitasov and others added 2 commits July 13, 2026 14:00
…ites CI jobs

Address the review comments:

- Fixtures are now MANDATORY (same approach for big and small responses):
  every response body lives in tests/fixtures/*.json; the harness gains
  load_fixture(__file__, 'name.json', **overrides) (positional-only params so
  overrides may use any record field name, deep-copied so bases stay pristine).
  The jira suite and the skill skeleton are converted.

- Harness unit tests and connector suites now run as SEPARATE CI jobs for
  clean results: harness_plugin grows --meta-only / --suites-only flags;
  components.py splits the component into connector-tests-harness (--meta-only)
  and connector-mock-tests (--suites-only) with a new registry-level
  triggered_by mechanism in changed.py so they co-trigger (suites consume the
  harness; merged gate coverage must include the meta share). ci.yml passes
  matrix.entry.pytest_args through to pytest. Both jobs' Cobertura reports
  merge under connector-tests-harness at the gate (95.4% locally).

- README: new 'How a nocode connector is tested' section answering the review
  question — the manifest IS the implementation, so the unit under test is
  connector.yaml executed in-process by the CDK with HTTP as the test boundary.

Local: meta-only 9 passed; suites-only 11 passed + 1 skipped; merged gate PASS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/ingestion/tests/connectors/connector_tests/source.py (1)

43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Specify encoding="utf-8" on the open() call.

YAML manifests should always be read as UTF-8. Without an explicit encoding, the platform default (e.g., cp1252 on Windows) could cause decode failures on manifests containing non-ASCII characters.

♻️ Proposed fix
-    with open(connector_dir(connector_path) / "connector.yaml") as f:
+    with open(connector_dir(connector_path) / "connector.yaml", encoding="utf-8") as f:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/tests/connectors/connector_tests/source.py` around lines 43 -
44, Update the open() call in the connector manifest loader to explicitly use
UTF-8 when reading connector.yaml, while preserving the existing yaml.safe_load
flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/ingestion/tests/connectors/connector_tests/source.py`:
- Around line 43-44: Update the open() call in the connector manifest loader to
explicitly use UTF-8 when reading connector.yaml, while preserving the existing
yaml.safe_load flow.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a1103470-c1cf-477e-86f9-319bd3d53d11

📥 Commits

Reviewing files that changed from the base of the PR and between 61899cf and ce51471.

📒 Files selected for processing (19)
  • .claude/skills/connector/workflows/create.md
  • .claude/skills/connector/workflows/test.md
  • .claude/skills/connector/workflows/validate.md
  • .github/workflows/ci.yml
  • scripts/ci/changed.py
  • scripts/ci/components.py
  • src/ingestion/connectors/task-tracking/jira/tests/fixtures/discovery_project.json
  • src/ingestion/connectors/task-tracking/jira/tests/fixtures/issue.json
  • src/ingestion/connectors/task-tracking/jira/tests/fixtures/project.json
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_projects.py
  • src/ingestion/tests/connectors/README.md
  • src/ingestion/tests/connectors/connector_tests/__init__.py
  • src/ingestion/tests/connectors/connector_tests/fixtures.py
  • src/ingestion/tests/connectors/connector_tests/plugin.py
  • src/ingestion/tests/connectors/connector_tests/source.py
  • src/ingestion/tests/connectors/harness_plugin.py
  • src/ingestion/tests/connectors/meta/test_harness.py
  • src/ingestion/tests/connectors/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/ingestion/tests/connectors/pyproject.toml
  • src/ingestion/tests/connectors/connector_tests/init.py
  • src/ingestion/tests/connectors/harness_plugin.py
  • .claude/skills/connector/workflows/validate.md
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_projects.py
  • .claude/skills/connector/workflows/create.md
  • .claude/skills/connector/workflows/test.md
  • src/ingestion/connectors/task-tracking/jira/tests/test_jira_issue_keys.py

…ver=False)

The split suites job measures only connector_tests files, which bucket to
connector-tests-harness at the gate — so connector-mock-tests itself never has
measured lines and the gate's --require set flagged it as a missing report
(Overall: FAIL, missing-reports: 1) while every job was green.

Give python entries the same cover flag rust/dotnet already have: cover=False
keeps the job running and uploading its Cobertura (it still merges into the
harness component's coverage) but removes the component from --require.
Verified locally: gate PASS with the corrected require set, FAIL reproduced
with the old one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
@mitasovr
mitasovr requested a review from ktursunov July 13, 2026 11:35

@ktursunov ktursunov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zoom metrics

@mitasovr
mitasovr merged commit 40aca32 into constructorfabric:main Jul 13, 2026
34 checks passed
@ktursunov ktursunov mentioned this pull request Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests for the Jira connector Tests for Airbyte connectors

2 participants