Skip to content

test: add IntegrationDependency integration tests#347

Open
zongqichen wants to merge 2 commits intofeat/integration-dependenciesfrom
feat/integration-dependencies-tests
Open

test: add IntegrationDependency integration tests#347
zongqichen wants to merge 2 commits intofeat/integration-dependenciesfrom
feat/integration-dependencies-tests

Conversation

@zongqichen
Copy link
Contributor

@zongqichen zongqichen commented Feb 2, 2026

Summary

  • Add external Data Product test packages (test-sai-supplier-v1, test-s4-supplier-v1)
  • Add integration tests for IntegrationDependency generation with @ORD.Extensions
  • Add integration tests for cdsrc customization of IntegrationDependency
  • Add integration tests for custom.ord.json merging of IntegrationDependency

Test plan

  • Run npm run test:integration:build to verify IntegrationDependency tests pass
  • Run npm run test:integration:basic to verify basic-auth tests still pass
  • Run npm run test:integration:mtls to verify mTLS tests still pass

Dependencies

This PR should be merged after the base branch PR (feat/integration-dependencies) is merged.

@zongqichen
test: add IntegrationDependency integration tests
0081a5d 
- Add external Data Product test packages (test-sai-supplier-v1, test-s4-supplier-v1)
- Add integration tests for IntegrationDependency generation with @ORD.Extensions
- Add integration tests for cdsrc customization of IntegrationDependency
- Add integration tests for custom.ord.json merging of IntegrationDependency
@hyperspace-insights
Copy link
Contributor

hyperspace-insights bot commented Feb 2, 2026

Summary

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

Add Integration Tests for IntegrationDependency Feature

✅ Test: Comprehensive integration test suite for the IntegrationDependency generation feature.

Changes

This PR adds integration tests to verify the IntegrationDependency generation functionality across multiple scenarios:

Test Infrastructure

  • cds-build.test.js: Enhanced test runner to support custom CDS configuration files via CDS_CONFIG environment variable, enabling multiple test scenarios with different configurations

Test Data Products

Added two external Data Product test packages simulating SAP Intelligent Agriculture (SAI) and S4HANA integrations:

  • srv/imported/test-sai-supplier-v1/: Test SAI Supplier Data Product (v1.0.0) with external service definition and ORD ID
  • srv/imported/test-s4-supplier-v1/: Test S4 Supplier Data Product (v1.0.0) with external service definition and ORD ID
  • srv/test-service.cds: Updated to import and use both external Data Products
  • package.json: Added file dependencies for the test Data Product packages

Test Scenarios

1. Basic IntegrationDependency Generation (13 new tests)

  • Verifies automatic generation of IntegrationDependency resources from external Data Products
  • Tests correct generation of aspects with @ORD.Extensions annotations
  • Validates IntegrationDependency structure, including ordId, title, version, release status, visibility, and mandatory flag
  • Confirms aspect properties (title, description, apiResources with ordId and minVersion)

2. cdsrc Customization (3 new tests)

  • Tests customization of IntegrationDependency properties via .cdsrc.integrationDep.json
  • Validates properties can be overridden: title, version, releaseStatus, visibility, and description
  • Ensures aspects from @ORD.Extensions are preserved even with cdsrc customization

3. custom.ord.json Merging (3 new tests)

  • Tests patching IntegrationDependency via ord/custom.ord.json
  • Verifies specific properties can be patched (title, shortDescription, mandatory)
  • Confirms non-patched properties and aspects are preserved from generated resources

Test Configuration Files

  • .cdsrc.integrationDep.json: Configuration for testing cdsrc-based customization
  • .cdsrc.customord.json: Configuration pointing to custom.ord.json for testing merge functionality
  • ord/custom.ord.json: Custom ORD content for testing patching behavior
  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.17.46 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Summary Prompt: Default Prompt
  • Event Trigger: pull_request.opened
  • LLM: anthropic--claude-4.5-sonnet
  • Correlation ID: 893b4bd0-0059-11f1-924c-d2f2025a0498
  • Output Template: Default Template

💌 Have ideas or want to contribute? Create an issue and share your thoughts with us!
📑 Check out the documentation for more information.

Made with ❤️ by Hyperspace.

hyperspace-insights[bot]
hyperspace-insights bot reviewed Feb 2, 2026
View reviewed changes
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot left a comment

Choose a reason for hiding this comment

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

Summary

The PR adds comprehensive integration tests for IntegrationDependency generation. However, there are several instances where array elements are accessed without proper null/undefined checks in the test predicates, which could cause runtime errors if the data structure doesn't match expectations. These should be fixed with optional chaining to make the tests more robust.

PR Bot Information

Version: 1.17.46 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Correlation ID: 893b4bd0-0059-11f1-924c-d2f2025a0498
  • LLM: anthropic--claude-4.5-sonnet
  • Event Trigger: pull_request.opened

__tests__/integration/cds-build.test.js
const integrationDep = ordDocument.integrationDependencies[0];
expect(integrationDep.aspects).toHaveLength(2);

const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot Feb 2, 2026

Choose a reason for hiding this comment

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

Bug: Potential runtime error accessing array without null/undefined check

a.apiResources[0] is accessed without verifying that a.apiResources exists or has elements. If the aspect object doesn't have apiResources or it's empty, this will throw a TypeError.

Consider adding a safe navigation check:

Suggested change
const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
const saiAspect = integrationDep.aspects.find((a) => a.apiResources?.[0]?.ordId.includes("test.sai"));

Double-check suggestion before committing. Edit this comment for amendments.

Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

__tests__/integration/cds-build.test.js
expect(saiAspect.apiResources[0].ordId).toBe("test.sai:apiResource:Supplier:v1");
expect(saiAspect.apiResources[0].minVersion).toBe("1.0.0");

const s4Aspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.s4"));
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot Feb 2, 2026

Choose a reason for hiding this comment

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

Bug: Potential runtime error accessing array without null/undefined check

a.apiResources[0] is accessed without verifying that a.apiResources exists or has elements. If the aspect object doesn't have apiResources or it's empty, this will throw a TypeError.

Consider adding a safe navigation check:

Suggested change
const s4Aspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.s4"));
const s4Aspect = integrationDep.aspects.find((a) => a.apiResources?.[0]?.ordId.includes("test.s4"));

Double-check suggestion before committing. Edit this comment for amendments.

Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

__tests__/integration/cds-build.test.js
expect(saiAspect.title).toBe("Test SAI Supplier API");
expect(saiAspect.description).toBe("Integration with Test SAI Supplier Data Product");
expect(saiAspect.mandatory).toBe(false);
expect(saiAspect.apiResources[0].ordId).toBe("test.sai:apiResource:Supplier:v1");
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot Feb 2, 2026

Choose a reason for hiding this comment

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

Bug: Array accessed before null check

Line 157 accesses saiAspect.apiResources[0].ordId before line 153 verifies saiAspect is defined. If saiAspect is undefined (e.g., if the find at line 152 returns nothing), this will throw a TypeError.

The expect(saiAspect).toBeDefined() assertion at line 153 doesn't prevent the error—it would fail the test, but only after attempting the property access at line 157. Consider restructuring the test or adding guards.

Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

__tests__/integration/cds-build.test.js
const integrationDep = ordDocument.integrationDependencies[0];
expect(integrationDep.aspects).toHaveLength(2);

const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot Feb 2, 2026

Choose a reason for hiding this comment

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

Bug: Potential runtime error accessing array without null/undefined check

a.apiResources[0] is accessed without verifying that a.apiResources exists or has elements. If the aspect object doesn't have apiResources or it's empty, this will throw a TypeError.

Consider adding a safe navigation check:

Suggested change
const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
const saiAspect = integrationDep.aspects.find((a) => a.apiResources?.[0]?.ordId.includes("test.sai"));

Double-check suggestion before committing. Edit this comment for amendments.

Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

__tests__/integration/cds-build.test.js
const integrationDep = ordDocument.integrationDependencies[0];
expect(integrationDep.aspects).toHaveLength(2);

const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot Feb 2, 2026

Choose a reason for hiding this comment

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

Bug: Potential runtime error accessing array without null/undefined check

a.apiResources[0] is accessed without verifying that a.apiResources exists or has elements. If the aspect object doesn't have apiResources or it's empty, this will throw a TypeError.

Consider adding a safe navigation check:

Suggested change
const saiAspect = integrationDep.aspects.find((a) => a.apiResources[0].ordId.includes("test.sai"));
const saiAspect = integrationDep.aspects.find((a) => a.apiResources?.[0]?.ordId.includes("test.sai"));

Double-check suggestion before committing. Edit this comment for amendments.

Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyperspace-insights hyperspace-insights[bot] hyperspace-insights[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zongqichen