Skip to content

Add Integration Testing on ANS Service and HANA#138

Open
eric-pSAP wants to merge 9 commits into
mainfrom
integrationTests
Open

Add Integration Testing on ANS Service and HANA#138
eric-pSAP wants to merge 9 commits into
mainfrom
integrationTests

Conversation

@eric-pSAP

@eric-pSAP eric-pSAP commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Hybrid testing is now supported. Some bugs were discovered in testing that were also adjusted and integration tests were re-worked.

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

The PR is generally well-structured, adding hybrid testing support and reorganizing tests into logical describe groups. The main concerns are: the require placement for the HTTP client mock inside a describe block in the unit tests (fragile under module reset scenarios), and the intentional-but-undocumented behavioral change of always populating cds.notifications.local.types in production when kind === 'notify-to-rest', which previously was skipped. Please verify the production exposure of local type data is intentional before merging.

PR Bot Information

Version: 1.26.11

  • LLM: anthropic--claude-4.6-sonnet
  • File Content Strategy: Full file content
  • Event Trigger: pull_request.opened
  • Correlation ID: e84ef2b9-3f9c-40c2-bc00-2a231c6bf562

Comment thread tests/integration/bookshop.test.js Outdated
Comment thread tests/unit/srv/notifyToRest.test.js Outdated
Comment thread cds-plugin.js
@eric-pSAP
eric-pSAP marked this pull request as ready for review June 25, 2026 14:42
@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

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


Add Hybrid Testing Support for ANS Service and Refactor Integration Tests

New Features

✨ Introduces hybrid testing support, allowing integration tests to run against real ANS/HANA services (via cds bind) in addition to local development mode. Tests now branch their assertions based on whether they are running in hybrid/production mode or local dev mode.

Changes

  • cds-plugin.js: Removed the production flag guard that previously skipped notification type processing and cds.notifications population in production. Now, cds.notifications.local.types is always populated on startup regardless of environment. Simplified the initialization logic by removing the redundant non-production branch.

  • package.json: Added a test:hybrid script (cds bind --exec -- npx jest --silent) to enable running integration tests in hybrid mode against bound cloud services.

  • srv/notifyToRest.js: Changed executeHttpRequest import to use the full httpClient module object (instead of destructuring), enabling Jest to spy on it at the module level during testing.

  • tests/integration/bookshop.test.js: Significantly restructured tests into logical describe blocks (Service implementation, Startup, i18n, Sending notifications, Event emission, Model validation). Added isHybrid detection via CDS_ENV. Tests now conditionally assert against console log output (dev) or HTTP spy calls (hybrid/production), covering default notifications, typed notifications, event emission, and order submission flows.

  • tests/unit/srv/notifyToRest.test.js: Added module-level mocks for @sap-cloud-sdk/connectivity, @sap-cloud-sdk/http-client, and ../../../lib/utils. Added a new Error handling describe block with tests covering ANS error message propagation, 4xx unrecoverable error marking, and correct handling of 429 and 5xx responses as recoverable.


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

Version: 1.26.11

  • Event Trigger: pull_request.ready_for_review
  • Output Template: Default Template
  • Summary Prompt: Default Prompt
  • File Content Strategy: Full file content
  • LLM: anthropic--claude-4.6-sonnet
  • Correlation ID: b7fd14a5-3040-482e-837b-00d36ef04115

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

The PR introduces hybrid integration testing support with good overall structure. The main concerns are: "production" being folded into the isHybrid flag (misleading name and potentially wrong behavior), and cds.notifications now being populated unconditionally for all service kinds/environments where it was previously guarded — both of which should be explicitly confirmed as intentional.

PR Bot Information

Version: 1.26.11

  • LLM: anthropic--claude-4.6-sonnet
  • Correlation ID: b7fd14a5-3040-482e-837b-00d36ef04115
  • Event Trigger: pull_request.ready_for_review
  • File Content Strategy: Full file content

Comment thread cds-plugin.js
Comment thread tests/integration/bookshop.test.js Outdated
Comment thread tests/integration/bookshop.test.js Outdated
Comment thread tests/integration/bookshop.test.js Outdated
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