Skip to content

Fix TargetIDSet.IsEmpty and add regression test - #220

Open
AleksaMCode wants to merge 1 commit into
pgsty:mainfrom
AleksaMCode:fix/lambda-targetidset-isempty
Open

AleksaMCode wants to merge 1 commit into
pgsty:mainfrom
AleksaMCode:fix/lambda-targetidset-isempty

Conversation

@AleksaMCode

Copy link
Copy Markdown

Description

Fixes an inverted emptiness check in lambda target ID sets and adds a regression test.

  • Updated internal/config/lambda/event/targetidset.go:
    • TargetIDSet.IsEmpty() now correctly returns len(set) == 0.
  • Added TestTargetIDSetIsEmpty in:
    • internal/config/lambda/event/targetidset_test.go.

Motivation and Context

I discovered this while evaluating Silo as a MinIO replacement for my production CI/CD environment.

TargetIDSet.IsEmpty() was returning true for non-empty sets (len(set) != 0), which is logically incorrect and could cause future call sites to invert control flow. This PR fixes the behavior and adds a dedicated unit test so the bug cannot regress silently.

How to test this PR?

go test ./internal/config/lambda/event

Compatibility impact

No API, protocol, config key, metric, header, route, storage format, or module path changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • All commits are signed off (git commit -s) per the DCO
  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • make verifiers passes
  • Relevant package tests and make build pass
  • Compatibility and rollback impact documented
  • Internal documentation updated
  • Public documentation update opened in pgsty/silo.pgsty.com, if needed

Signed-off-by: AleksaMCode <aleksamcode@gmail.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