Skip to content

test(bdd): Gherkin/BDD e2e pilot on the existing Vitest runner#1038

Open
branarakic wants to merge 2 commits into
mainfrom
test/bdd-gherkin-pilot
Open

test(bdd): Gherkin/BDD e2e pilot on the existing Vitest runner#1038
branarakic wants to merge 2 commits into
mainfrom
test/bdd-gherkin-pilot

Conversation

@branarakic

Copy link
Copy Markdown
Contributor

What & why

A pilot evaluating Gherkin/BDD end-to-end tests for the DKG monorepo — describing system behaviour in human-readable Given/When/Thenbefore committing to it more broadly.

It deliberately runs on the project's existing Vitest runner via @amiceli/vitest-cucumber (peerDep vitest ^4.0.4, matches our ^4.0.18): one runner, one CI lane, no standalone Cucumber, no Jest typings.

Self-contained in a new bdd/ workspace package — nothing existing is changed except adding the workspace member + lockfile entry.

What's included

Spec Purpose Runs
bdd/features/entity-predicate.feature Pure smoke spec — data-driven Scenario Outline over the OT-RFC-43/44 dkg:entity ↔ legacy dkg:rootEntity dual-read invariant (exercises the real packages/core/src/entity-predicate.ts) The package's test script → runs under turbo test (CI). ✅ 27 step-tests green
bdd/features/ka-lifecycle.feature The KA lifecycle create → write → finalize → promote → publish as readable behaviour; mirrors devnet/v10-core-flows fullPublish() @devnet, tag-gated → self-skips cleanly with no cluster; run via test:devnet
pnpm --filter @origintrail-official/dkg-bdd test        # smoke (CI default)
pnpm --filter @origintrail-official/dkg-bdd test:all    # both (devnet self-skips)
pnpm --filter @origintrail-official/dkg-bdd test:devnet # needs ./scripts/devnet.sh start 6

Verification

  • Smoke spec verified green through turbo test (the CI path), not just direct vitest.
  • The @devnet spec's every route / request payload / response field was cross-checked against the live daemon handlers (assertion.ts, memory.ts, status.ts) and matches fullPublish()write→written, promote→promotedCount, finalize→merkleRoot/eip712Digest, publish→kaId/status, /api/status→identityId. It has not been executed against a live cluster in this pilot.
  • An adversarial review pass hardened gating, scenario-state isolation, and auth.token parsing (matches the harness's #-comment handling).

Scope / how to extend

This is intentionally narrow — a working template + proposal (see bdd/README.md for the full rationale and tradeoffs). It does not rewrite the 9 existing devnet Vitest suites. If we expand, the highest readable-spec value is the KA-routes unification and conviction-staking flows. Feedback on the ergonomics welcome.

🤖 Generated with Claude Code

Self-contained `bdd/` package that runs Gherkin .feature specs via
@amiceli/vitest-cucumber (Vitest-native — one runner, one CI lane, no
standalone Cucumber):

- entity-predicate.feature: pure smoke spec over the OT-RFC-43/44 dual-read
  invariant in packages/core/src/entity-predicate.ts. Exposed as the package's
  `test` script so it runs under the standard `turbo test` lane (27 step-tests).
- ka-lifecycle.feature: @DevNet KA lifecycle (create -> write -> finalize ->
  promote -> publish) mirroring devnet/v10-core-flows fullPublish(); tag-gated
  to skip cleanly when no .devnet is present (run via `test:devnet`).

Route/payload surface of the @DevNet spec cross-checked against the live
daemon handlers (assertion.ts, memory.ts, status.ts). See bdd/README.md for
the rationale, tradeoffs, and how to extend.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread bdd/package.json
"test:all": "vitest run --config vitest.config.ts"
},
"devDependencies": {
"@amiceli/vitest-cucumber": "^6.5.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Bug: This PR adds a new external dependency but does not update pnpm-lock.yaml. All of the repo's CI jobs install with pnpm install --frozen-lockfile, so they will fail as soon as this package lands. Regenerate and commit the lockfile alongside this manifest change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This looks like a false positive: pnpm-lock.yaml was committed in the initial commit (3 @amiceli/vitest-cucumber entries) and pnpm install --frozen-lockfile passes (exit 0) on this branch. 9448c877e adds the new @origintrail-official/dkg-core workspace dep with the lockfile updated alongside; frozen install still passes.

Comment thread bdd/steps/entity-predicate.steps.ts Outdated
import {
isAssertionEntityPredicate,
isEntityPredicate,
} from '../../packages/core/src/entity-predicate.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Issue: Importing ../../packages/core/src/... directly bypasses the workspace package graph, so Turbo/PNPM do not see bdd as depending on dkg-core. That can let turbo test reuse a cached smoke-spec result after packages/core/src/entity-predicate.ts changes, which defeats the purpose of this regression check. Prefer a declared workspace dependency on @origintrail-official/dkg-core and import through its public export.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 9448c877e. Now imports the helpers via the public @origintrail-official/dkg-core export, declared as a workspace:* dependency, plus a bdd/turbo.json setting test to dependsOn: ["^build"]. turbo run test --filter=@origintrail-official/dkg-bdd --dry now shows @origintrail-official/dkg-core#build as a dependency, so the smoke cache invalidates when that code changes.

Comment thread bdd/support/world.ts
if (!apiPort) return { available: false, reason: 'no apiPort in node config' };

return {
available: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Issue: detectDevnet() marks the devnet as available based only on .devnet/node1/config.json, so a stopped but still-provisioned cluster will be treated as runnable. In that state test:all will include the @devnet scenario and then fail in BeforeAllScenarios, even though the README says it should self-skip without a cluster. Gate availability on a live status probe as well, or downgrade an offline node to a skip instead of an error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9448c877e. Availability is now gated on a live /api/status probe (3s timeout), not just on-disk provisioning. Verified by simulation: with a provisioned-but-stopped node (config present, port dead), test:all skips cleanly (@devnet scenarios skipped — node1 provisioned but not responding ...) and exits 0 instead of failing in BeforeAllScenarios.

- Import isEntityPredicate/isAssertionEntityPredicate via the public
  @origintrail-official/dkg-core export and declare it as a workspace
  dependency; add bdd/turbo.json so `test` dependsOn `^build`. Turbo now sees
  bdd -> dkg-core and invalidates the smoke-spec cache when that code changes
  (the relative `../../packages/core/src` import bypassed the package graph and
  could serve a stale cached pass).

- Gate @DevNet availability on a live /api/status probe (with a 3s timeout),
  not just on-disk provisioning, so a stopped-but-provisioned cluster skips
  cleanly instead of failing in BeforeAllScenarios. Verified by simulation.

The review's "pnpm-lock.yaml not updated" finding is a false positive: the
lockfile was committed in the initial commit (3 @amiceli entries) and
`pnpm install --frozen-lockfile` passes (exit 0).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codex review skipped: filtered diff is 20685 lines (cap: 5,000). Please consider splitting this into smaller PRs for reviewability.

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