Skip to content

test(cypress): add Helcim refunds and mandates coverage#13157

Open
venkatakarthikm-maker wants to merge 2 commits into
mainfrom
helcim-mandate-skip
Open

test(cypress): add Helcim refunds and mandates coverage#13157
venkatakarthikm-maker wants to merge 2 commits into
mainfrom
helcim-mandate-skip

Conversation

@venkatakarthikm-maker

@venkatakarthikm-maker venkatakarthikm-maker commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

Fixes #13158

@venkatakarthikm-maker venkatakarthikm-maker self-assigned this Jul 3, 2026
@venkatakarthikm-maker venkatakarthikm-maker requested a review from a team as a code owner July 3, 2026 09:50
@semanticdiff-com

semanticdiff-com Bot commented Jul 3, 2026

Copy link
Copy Markdown

@XyneSpaces

Copy link
Copy Markdown

💡 Document the Helcim test exclusion rationale in code comments

The PR adds excellent inline documentation in Utils.js explaining the Helcim sandbox limitation. Consider also adding a brief code comment in the mandate test file (20-MandatesUsingPMID.cy.js) referencing the same limitation, as mandates are excluded for a different reason than refunds.

Also verify that the MANDATE_ID_TEST exclusion is warranted — the PR description only mentions refunds, but mandates are also being excluded. Document the mandate exclusion reason if it differs from refunds.

@HS-QUALIBOT HS-QUALIBOT changed the title Helcim refunds and mandates coverage test(cypress): add Helcim refunds and mandates coverage Jul 3, 2026
@XyneSpaces

Copy link
Copy Markdown

Code Review: Helcim Test Exclusions

Assessment

Clean, well-documented test configuration changes that appropriately exclude Helcim from refund and mandate tests due to sandbox limitations.

Findings

1. [nit] Consistent comment format

The comments explaining Helcim's sandbox limitation are thorough and include the external reference to hyperswitch-prism tests. Good practice!

2. [nit] Code duplication in skip logic

The skip logic pattern is repeated across multiple test files. Consider extracting this into a shared utility function in Utils.js:

// Potential helper in Utils.js
export function skipIfExcluded(connectorId, exclusionList) {
  return cy.task("getGlobalState").then((state) => {
    const globalState = new State(state);
    if (shouldExcludeConnector(globalState.get("connectorId"), exclusionList)) {
      this.skip();
    }
    return globalState;
  });
}

This isn't blocking since it's test code and follows existing patterns in the codebase.

Verdict

LGTM. The exclusion is well-justified with documentation links, and the implementation correctly handles the Cypress async patterns with function binding for this.skip() access.

@XyneSpaces

Copy link
Copy Markdown

Automated Code Review Results

Team: Test Automation (Cypress)

Findings Summary

✅ Well-Structured Connector Exclusion Pattern
This PR demonstrates the correct approach for handling connector-specific limitations in tests.

Review Notes

📝 Clear Documentation of Exclusions
The comment in Utils.js explaining why Helcim is excluded from refund tests is excellent:

"Helcim sandbox limitation: transactions never reach closed/settled batch state"

This pattern should be used for all future connector exclusions.

🔍 Implementation Review

Utils.js (line ~70):

EXCLUDE: {
  REFUND: ["helcim"],
  MANDATE_ID_TEST: [],
  // ...
}

Clean structure allowing future exclusions without structural changes.

09-RefundPayment.cy.js (before hook):

if (utils.shouldExcludeConnector(globalState, "REFUND")) {
  cy.log("Skipping refund test for excluded connector");
  return;
}

💡 Enhancement Suggestion
Consider adding a helper that logs why a connector is excluded, reading from the comment/documentation:

utils.skipWithReason(globalState, "REFUND"); 
// Logs: "Skipping: Helcim sandbox limitation: transactions never reach closed/settled batch state"

⚠️ Future Consideration
As more connectors are added to exclusion lists, consider moving the configuration to a separate connector-exclusions.config.js file for easier maintenance.

Overall Assessment: Proper handling of third-party limitations. Clean, maintainable code. Approved.

@XyneSpaces

Copy link
Copy Markdown

Review Summary

Classification: Analytics team — Cypress test configuration
Risk: Low

✅ No blocking issues


Findings

This PR correctly excludes Helcim from refund and mandate tests due to documented sandbox limitations. The approach is consistent with existing patterns in the codebase:

  1. EXCLUDE.REFUND list extension — Properly documented with inline comments explaining Helcim's sandbox behavior (transactions never reaching closed batch state).

  2. Consistent skip logic — All affected test files use the same shouldExcludeConnector pattern with this.skip() in the before hook.

  3. Cross-reference validation — Links to hyperswitch-prism test file confirms team alignment on the limitation.


Minor Observation

The MANDATE_ID_TEST exclusion list addition in 20-MandatesUsingPMID.cy.js lacks the detailed inline comment present in the refund exclusions. While the pattern is clear from context, consider adding a brief comment explaining why Helcim is excluded from mandate tests for consistency.

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

Labels

S-test-ready Status: This PR is ready for cypress-tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(cypress): add Helcim refunds and mandates cypress coverage

2 participants