Skip to content

test: remove sqlite skips for queue-worker tracing (merge after cds spawn fix) - #467

Open
sjvans wants to merge 1 commit into
improve-tracing-and-test-infrastructurefrom
remove-sqlite-queue-worker-skips
Open

test: remove sqlite skips for queue-worker tracing (merge after cds spawn fix)#467
sjvans wants to merge 1 commit into
improve-tracing-and-test-infrastructurefrom
remove-sqlite-queue-worker-skips

Conversation

@sjvans

@sjvans sjvans commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Depends on

  • Base branch feat: trace queue worker transactions + structured span test infrastructure #465 (this is stacked on top of it)
  • CDS queue-spawn fix must be released first and @sap/cds bumped: cap/cds test/queue-spawn-sqlite-extended-tenant — removes the sqlite setTimeout bypass in libx/queue/processing.js (_begin = cds.spawn.bind(cds)) and fixes the actual single-writer deadlock by wrapping ExtendedModels.model4() in cds.tx() inside lib/req/spawn.js.

What

Removes the 5 sqlite skip guards added in #465. Once cds routes the sqlite queue worker through cds.spawn, the cds.spawn - run task root span and its child tx spans appear on sqlite, so these suites verify the full path on sqlite (not just HANA):

  • tracing-scheduled.test.js
  • tracing-outboxed-batch.test.js
  • tracing-messaging-inboxed.test.js
  • tracing-messaging-outboxed-and-inboxed.test.js
  • tracing-messaging-persistent-outbox.test.js

Verified

Locally against a workspace-linked @sap/cds at test/queue-spawn-sqlite-extended-tenant: all 5 suites pass on sqlite.

Note: the AsyncResource.bind approach in cap/cds#6594 does not make these pass — only the test/queue-spawn-sqlite-extended-tenant fix (spawn everywhere + cds.tx-wrapped model4) does. Prefer that fix.

Do not merge until

@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:

⚠️ Warnings:

  • Could not get issue cap-js/telemetry#6594. Status: 404 - UnknownObjectException

Remove SQLite Skip Guards for Queue-Worker Tracing Tests

Test

🧪 Removes the temporary SQLite skip guards from 5 queue-worker tracing test suites that were added in #465 as a workaround for a cds.spawn routing issue on SQLite. Now that the upstream @sap/cds fix routes the SQLite queue worker through cds.spawn (instead of a raw setTimeout bypass), the cds.spawn - run task root span and its child tx spans are correctly emitted on SQLite, allowing these suites to run and verify the full tracing path on SQLite (not just HANA).

Changes

  • test/tracing-scheduled.test.js: Removed the SQLite skip block that prevented queue-worker span assertions from running when db.kind === 'sqlite'.
  • test/tracing-outboxed-batch.test.js: Removed the SQLite skip block for the outboxed batch chunk-size fan-out tracing test.
  • test/tracing-messaging-inboxed.test.js: Removed the unused cds import and the SQLite conditional skip guard.
  • test/tracing-messaging-outboxed-and-inboxed.test.js: Removed the unused cds import and the SQLite conditional skip guard.
  • test/tracing-messaging-persistent-outbox.test.js: Removed the unused cds import and the SQLite conditional skip guard.

Jira Issues (ONLY add this section if JIRA issues are available)

N/A

Related PRs

  • Stacked on top of #465: feat: wrap cds.Service.prototype.tx for queue worker tracing + structured in-memory span test infrastructure.

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.29.18

@hyperspace-pr-bot hyperspace-pr-bot Bot 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.

⚠️ Warnings:

  • Could not get issue cap-js/telemetry#6594. Status: 404 - UnknownObjectException

The changes are straightforward — they remove temporary sqlite skip guards from five test files. Looking at the diff carefully:

  1. In tracing-messaging-inboxed.test.js and tracing-messaging-outboxed-and-inboxed.test.js, the const cds = require('@sap/cds') line is being removed along with the sqlite guard. However, cds is not used anywhere else in those files — so the removal is clean.

  2. In tracing-outboxed-batch.test.js and tracing-scheduled.test.js, cds is still used in the file (for cds.test(...), cds.version, cds.connect.to, etc.), so the removal of the guard is fine and does not leave a dangling require.

  3. In tracing-messaging-persistent-outbox.test.js, same as the messaging files — cds is not directly used in the file body outside what was removed.

The PR description is clear that this is intentionally gated on an upstream CDS fix and the checklist items are unresolved. There are no logic errors, bugs, or maintainability issues introduced by the diff itself — all the removals are consistent and correct.

This is a clean, intentional removal of temporary workaround guards. The diff is correct and all five files are consistent — no issues to flag. The open checklist items in the PR description (#465 merged and @sap/cds bumped) are the only remaining gates before merge, which are appropriately tracked in the PR description itself.

PR Bot Information

Version: 1.29.18

  • LLM: anthropic--claude-4.6-sonnet
  • Event Trigger: pull_request.opened
  • Correlation ID: 9e2d0e20-919a-11f1-97de-effae0c5a313
  • File Content Strategy: Full file content

@sjvans
sjvans force-pushed the improve-tracing-and-test-infrastructure branch from 60e48a9 to b281c8f Compare August 6, 2026 13:37
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch from 9595503 to 31e0441 Compare August 6, 2026 13:38
@sjvans
sjvans force-pushed the improve-tracing-and-test-infrastructure branch from b281c8f to 9b88203 Compare August 6, 2026 13:41
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch 3 times, most recently from 81c9cd9 to 5eb33aa Compare August 6, 2026 14:04
@sjvans
sjvans force-pushed the improve-tracing-and-test-infrastructure branch from 89d1e87 to 29710d5 Compare August 7, 2026 08:05
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch from 5eb33aa to 5fe19e4 Compare August 7, 2026 08:05
@sjvans
sjvans force-pushed the improve-tracing-and-test-infrastructure branch from 29710d5 to 3983e27 Compare August 7, 2026 08:55
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch from 5fe19e4 to 179982c Compare August 7, 2026 08:55
@sjvans
sjvans force-pushed the improve-tracing-and-test-infrastructure branch from 3983e27 to 74d924a Compare August 7, 2026 13:51
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch from 179982c to 91c38f7 Compare August 7, 2026 13:52
Removes the sqlite skip guards added while the cds queue-spawn fix was pending.
With cds routing the sqlite queue worker through cds.spawn (cap/cds branch
test/queue-spawn-sqlite-extended-tenant: removes the setTimeout bypass and fixes
the underlying deadlock by wrapping ExtendedModels.model4() in cds.tx()), the
'cds.spawn - run task' root span and its child tx spans now appear on sqlite too.

Merge only after that cds fix is released and the @sap/cds dependency is bumped.
@sjvans
sjvans force-pushed the remove-sqlite-queue-worker-skips branch from 91c38f7 to d1812a1 Compare August 7, 2026 14:03
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.

1 participant