Skip to content

test(pyamber): add unit tests for Operator base class#4735

Merged
aglinxinyuan merged 3 commits into
apache:mainfrom
Yicong-Huang:test-pyamber-operator
May 3, 2026
Merged

test(pyamber): add unit tests for Operator base class#4735
aglinxinyuan merged 3 commits into
apache:mainfrom
Yicong-Huang:test-pyamber-operator

Conversation

@Yicong-Huang
Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds pytest coverage for amber/src/main/python/core/models/operator.py. The Operator abstract base class had no dedicated spec.

Any related issues, documentation, discussions?

Closes #4733.

Bug pinned in the spec with an explanatory comment (filed separately as a Bug issue): SourceOperator declares __internal_is_source = True at class level, but Python name-mangles that into _SourceOperator__internal_is_source while Operator.is_source reads self._Operator__internal_is_source. The two are different attributes, so a fresh SourceOperator subclass instance reports is_source=False until ExecutorManager.initialize_executor invokes the explicit setter — making the class-level declaration effectively dead code.

How was this PR tested?

cd amber/src/main/python
ruff check core/models/test_operator.py
ruff format --check core/models/test_operator.py
python -m pytest core/models/test_operator.py

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (claude-opus-4-7)

Closes apache#4733

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.52%. Comparing base (e99577b) to head (db30d54).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4735      +/-   ##
============================================
- Coverage     42.96%   42.52%   -0.45%     
+ Complexity     2035     2028       -7     
============================================
  Files           957      957              
  Lines         34077    34094      +17     
  Branches       3753     3753              
============================================
- Hits          14642    14499     -143     
- Misses        18659    18816     +157     
- Partials        776      779       +3     
Flag Coverage Δ
amber 40.93% <ø> (-0.07%) ⬇️
python 84.84% <ø> (+0.68%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new pytest spec file to exercise the Operator base class behaviors in core/models/operator.py, improving unit-level confidence around template decoding, source-ness flags, default lifecycle hooks, and batch-size validation (per #4733).

Changes:

  • Add tests for Operator.PythonTemplateDecoder (stdlib + injectable decoder, caching + eviction).
  • Add tests for Operator.is_source getter/setter and document the current SourceOperator name-mangling bug.
  • Add tests for Operator default no-op methods and BatchOperator._validate_batch_size.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread amber/src/main/python/core/models/test_operator.py Outdated
Comment thread amber/src/main/python/core/models/test_operator.py
…erage

- Replace the "is_source=False as contract" pin with two tests: one
  that asserts the underlying name-mangled attribute divergence, and
  one strict xfail that asserts the *intended* contract
  (`is_source is True` for SourceOperator subclasses), so a future
  bugfix flips xfail to XPASS instead of looking like a regression.
- Add TestTableOperator covering process_tuple buffering, on_finish
  building a Table from buffered tuples and routing it to
  process_table, the empty-buffer path, and per-port buffer isolation.
@aglinxinyuan aglinxinyuan enabled auto-merge (squash) May 3, 2026 03:01
@aglinxinyuan aglinxinyuan merged commit 1c3ec67 into apache:main May 3, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests for pyamber Operator base class

4 participants