Skip to content

test(frontend): add unit tests for serializePortIdentity - #6938

Open
rbelavadi wants to merge 2 commits into
apache:mainfrom
rbelavadi:test/6631-port-identity-serde-unit-tests
Open

test(frontend): add unit tests for serializePortIdentity#6938
rbelavadi wants to merge 2 commits into
apache:mainfrom
rbelavadi:test/6631-port-identity-serde-unit-tests

Conversation

@rbelavadi

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds port-identity-serde.spec.ts to cover serializePortIdentity, a previously untested pure utility function in common/util/ that serializes a PortIdentity into the stable string key id_internal.

This format is one half of a cross-language contract: the backend produces the identical string in PortIdentityKeySerializer.portIdToString and reads it back by splitting on "_" in PortIdentityKeyDeserializer. A format mismatch on either side would cause the frontend's schema lookup to silently return undefined rather than fail, so the exact output is worth pinning.

Covers four groups of behavior:

  • Format: exact key for external and internal ports, including the id: 0 falsy-number boundary and large and negative ids passing through unvalidated. These mirror PortIdentityKeySerializerSpec.scala's case table exactly, so both ends of the contract are now pinned by matching assertions.
  • Distinctness: ports differing only by id, or only by internal, cannot collide on the same key.
  • Stability: equal inputs always serialize identically.
  • Real usage: the key works for object lookup when generated independently at write and read time, and remains splittable into exactly two fields for the backend deserializer.

No production code was changed.

Any related issues, documentation, discussions?

Closes #6631

How was this PR tested?

Added 12 new unit tests in port-identity-serde.spec.ts covering all behavior described in the issue. Ran locally via ng test --include src/app/common/util/port-identity-serde.spec.ts --watch=false; all 12 pass. Full suite also green via yarn test (3817 passed, 2 skipped, 198 files).

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

Generated-by: Claude Code (Claude Opus 4.8)

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Jul 27, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.91%. Comparing base (a351f44) to head (84bf928).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6938      +/-   ##
============================================
+ Coverage     77.30%   82.91%   +5.60%     
============================================
  Files          1161      328     -833     
  Lines         45922    20135   -25787     
  Branches       5101     2905    -2196     
============================================
- Hits          35501    16695   -18806     
+ Misses         8840     2775    -6065     
+ Partials       1581      665     -916     
Flag Coverage Δ
access-control-service ?
agent-service ?
amber ?
computing-unit-managing-service ?
config-service ?
file-service ?
frontend 82.91% <ø> (+0.33%) ⬆️
notebook-migration-service ?
pyamber ?
workflow-compiling-service ?

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rbelavadi

Copy link
Copy Markdown
Contributor Author

/request-review @Ma77Ball

@xuang7 xuang7 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.

LGTM!

@xuang7
xuang7 enabled auto-merge July 27, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit test coverage for serializePortIdentity

4 participants