Skip to content

feat(event): add shorthand dedupe regression test#72

Draft
overtrue wants to merge 1 commit intomainfrom
codex/event-shorthand-dedupe-gap
Draft

feat(event): add shorthand dedupe regression test#72
overtrue wants to merge 1 commit intomainfrom
codex/event-shorthand-dedupe-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Summary

This change adds a focused regression test for the event add shorthand-normalization path.

Users can pass shorthand event names such as put, get, or delete, and the CLI now has explicit coverage for the case where those shorthands are mixed with their canonical S3 event names in the same input. The expected behavior is to normalize both forms to the same persisted event name and deduplicate the final notification event list.

Problem

The recent event normalization fix added support for shorthand event names before persisting notification rules, but the tests only proved that shorthand values normalize correctly on their own. They did not cover the user-facing case where shorthand and canonical values are combined in one command.

Without that regression coverage, a future refactor could reintroduce duplicate persisted events such as both put and s3:ObjectCreated:* surviving as separate entries after parsing.

Root Cause

The missing test path was the overlap between normalization and deduplication. Existing tests covered:

  • default event behavior and simple deduplication
  • shorthand normalization on its own

They did not cover normalization producing values that should then collapse against already-canonical inputs.

Fix

Add a unit test in crates/cli/src/commands/event.rs that mixes shorthand and canonical event names for created, accessed, and removed events, then asserts that the parsed list contains only the canonical deduplicated values.

Validation

The following checks passed:

  • cargo fmt --all --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant