No test exercises the table-format instanceNames.set, so dropping it would silently reopen #1976 for table-format sinks only

Open Beginner friendly
  • Is something wrong?
#2,067 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript
Domain
testing

Research direction

Read the table-format fixture in test/core/sink-registry.test.js:280 and the hostile-instance-name tests in test/core/sink-hostile-instance-name.test.js. Add a table-format case with a throwing instanceName accessor and verify sinkInstanceName returns the validated key; the test should fail if the instanceNames.set at src/core/registry/sinks.js:534 is removed.

Written by the indexing model from the issue text.

Description

Source: #2057 at fb6035d0448e2e44b05deabbe3387fb404265070
Location: src/core/registry/sinks.js:534
Severity: minor

Evidence

Verified at head fb6035d0: instanceNames.set(handle, instanceName) is written at src/core/registry/sinks.js:431 (blob/request) and :534 (table-format), and every fixture in test/core/sink-hostile-instance-name.test.js instantiates kind 'request', so only :431 is pinned (round 1's mutation run killed the :431 drop 0/8; nothing kills a :534 drop). An adjudicator probe instantiating a real table-format sink through createSinkRegistry and installing a throwing instanceName accessor confirms sinkInstanceName answers the validated key 'table-inst', so the line is behaviorally correct today; the gap is only that no test would catch its removal, and a removal is invisible for honest handles because the guarded fallback returns the same name.

Why deferral is safe

No incorrect behavior exists at the head, confirmed by direct execution, so this is a coverage guard against a future regression, not a defect. The line is a verbatim one-line mirror sitting directly after its handles.set, exactly like the covered call. A table-format hostile fixture is a larger lift than #1976's smallest change warrants in this PR, and the existing table-format instantiate fixture in test/core/sink-registry.test.js:280 makes the follow-up cheap. Backlog only; not admitted for automatic repair.

Acceptance condition

Add one table-format case to test/core/sink-hostile-instance-name.test.js (the fixture shape at test/core/sink-registry.test.js:280 suffices): with a throwing instanceName accessor on the table-format handle, sinkInstanceName answers the validated key, and deleting the set at sinks.js:534 makes the case fail.

Review preference; backlog only. A human may explicitly delegate it with neutral:fix.

Dominant language
JavaScript
Stars
11
Forks
0
Avg merge
2h 55m
Merged PRs (30d)
294

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from hyparam/hypaware

All issues in hyparam/hypaware

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.