Skip to content

Add BDD test suite covering V2 SDK functionality#66

Open
b3y0urs3lf wants to merge 2 commits into
mainfrom
bdd-phase-0
Open

Add BDD test suite covering V2 SDK functionality#66
b3y0urs3lf wants to merge 2 commits into
mainfrom
bdd-phase-0

Conversation

@b3y0urs3lf
Copy link
Copy Markdown
Contributor

Ports the TypeScript SDK's BDD coverage to Java, mirroring scenarios across aggregator communication, token minting/transfer/verification, splits, payments, sharding, CBOR envelope strictness, and inclusion proofs. Covers the V2 PR chain (#53, #54, #59, #61, #62, #63, #65) end-to-end against both the hermetic in-process aggregator and the real BFT-shard topology via subscription proxy.

  • 49 Cucumber feature files (~149 scenario declarations)
  • 33 step-definition classes wired through PicoContainer
  • Test-only support clients: StrictTestAggregatorClient, ForgivingTestAggregatorClient, ShardAwareAggregatorClient, SimulatedShardPool, NametagRegistry, TokenTree(Builder)
  • Tag-based runtime gating: @hermetic-only, @bft-shard-only, @multi-shard-only, @pending-src-cleanup, @stateful, @fresh-aggregator, @slow
  • Env wiring: AGGREGATOR_URL, AGGREGATOR_API_KEY, TRUST_BASE_PATH
  • JUnit Platform Suite runner with package selection

Ports the TypeScript SDK's BDD coverage to Java, mirroring scenarios
across aggregator communication, token minting/transfer/verification,
splits, payments, sharding, CBOR envelope strictness, and inclusion
proofs. Covers the V2 PR chain (#53, #54, #59, #61, #62, #63, #65)
end-to-end against both the hermetic in-process aggregator and the
real BFT-shard topology via subscription proxy.

- 49 Cucumber feature files (~149 scenario declarations)
- 33 step-definition classes wired through PicoContainer
- Test-only support clients: StrictTestAggregatorClient,
  ForgivingTestAggregatorClient, ShardAwareAggregatorClient,
  SimulatedShardPool, NametagRegistry, TokenTree(Builder)
- Tag-based runtime gating: @hermetic-only, @bft-shard-only,
  @multi-shard-only, @pending-src-cleanup, @stateful,
  @fresh-aggregator, @slow
- Env wiring: AGGREGATOR_URL, AGGREGATOR_API_KEY, TRUST_BASE_PATH
- JUnit Platform Suite runner with package selection

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a comprehensive Cucumber-based end-to-end testing suite for the Unicity SDK, including new test tasks in build.gradle.kts, a robust test context management system, and numerous step definitions covering token lifecycle, addressing, serialization, and security scenarios. The reviewer provided actionable feedback regarding the Gradle configuration to avoid overwriting system properties and suggested safer shutdown patterns for the ExecutorService in the shard load tests.

Comment thread build.gradle.kts
Comment on lines +191 to +192
systemProperty("cucumber.junit-platform.naming-strategy", "long")
systemProperties = System.getProperties().toMap() as Map<String, Any>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Assigning to systemProperties replaces the entire map of system properties for the test task. This causes the cucumber.junit-platform.naming-strategy property set on the previous line to be lost unless it is also present in the host JVM's system properties. Use the systemProperties() method instead to append properties to the existing map. This pattern is repeated in several other BDD tasks (bddNametag, bddSlow, etc.).

    systemProperty("cucumber.junit-platform.naming-strategy", "long")
    systemProperties(System.getProperties().toMap() as Map<String, Any>)

5, TimeUnit.MINUTES);
} finally {
exec.shutdown();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The ExecutorService is shut down using shutdown(), which allows existing tasks to continue running. In a test environment, especially if a timeout occurs (line 166), it is safer to use shutdownNow() to attempt to stop active tasks and prevent resource leaks or interference with subsequent tests. Additionally, calling awaitTermination ensures the pool is fully cleaned up before the method returns.

        exec.shutdownNow();
        exec.awaitTermination(5, TimeUnit.SECONDS);

aggregator-go removed the finalized-duplicate lookup at submit time
(async v2): a re-spend of an already-finalized state now returns SUCCESS
at submit and is rejected one layer later, at inclusion-proof
verification, with TRANSACTION_HASH_MISMATCH. Double-spend safety is
unchanged — only the rejection layer moved (submit -> proof).

- Default hermetic aggregator switched from StrictTestAggregatorClient
  to ForgivingTestAggregatorClient so CI mirrors the merged v2 contract
  and actually exercises the proof-time rejection path.
- New shared StepHelper.assertRespendRejected: a re-spend must never
  finalize — accepted as SUCCESS then rejected at proof with
  TRANSACTION_HASH_MISMATCH, or rejected at submit with STATE_ID_EXISTS
  (legacy). The invariant holds on every aggregator build and topology.
- Converted the 14 re-spend scenarios (token-4level-owner-actions x8,
  token-transfer-edge-cases, token-split-transfer x4, token-split-advanced)
  off the v1 submit-time STATE_ID_EXISTS assertion.
- STATE_ID_EXISTS now reserved for byte-identical idempotency tests.

Hermetic suite: 351 scenarios, 0 failures. Mirrors the TS SDK change
(state-transition-sdk-js#118).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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