Skip to content

Fix test to respect server 2.15 change to max_consumers behavior - #1623

Merged
scottf merged 1 commit into
mainfrom
2-15-max-consumers
Sep 3, 2026
Merged

scottf merged 1 commit into
mainfrom
2-15-max-consumers

Conversation

@scottf

@scottf scottf commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

The CI uses the latest (head) server. 2.15 has a change where creating a consumer when a stream does not set the max_consumers limits the number of consumers to 1000. This fixes a test that specifically creates more than 1024 consumer to test the paging ability of the js management getConsumers call.

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Sep 3, 2026

Copy link
Copy Markdown

Claude finished @scottf's task in 1m 0s —— View job


PR Review: Fix test to respect server 2.15 change to max_consumers behavior

Tasks

  • Gather context and read changed files
  • Understand the behavioral change being addressed
  • Review the fix for correctness and completeness

Summary

This is a targeted, minimal test fix. The change is correct.

Root cause: NATS server 2.15 introduced a stricter enforcement of max_consumers. Previously, TestingStreamContainer created a stream via createMemoryStream() with no explicit maxConsumers, which defaults to -1 (unlimited in older servers). Under server 2.15, the effective default appears to be lower than 1100, causing the test to fail when trying to add 600 + 500 = 1100 consumers.

Fix: Explicitly create the stream with maxConsumers(10000), giving the test ample headroom above the 1100 consumers it creates.


No Issues Found

The fix is clean:

  • Uses stream() and subject() helpers consistently with the rest of the test base
  • Preserves StorageType.Memory (consistent with the old createMemoryStream() path)
  • All tsc.stream references are correctly replaced with the local stream variable
  • The assertions (assertEquals(600, ...) and assertEquals(1100, ...)) are unchanged — the test logic itself is unaffected

The choice of 10000 provides generous headroom over the 1100 consumers the test actually creates, and is not so large as to be unreasonable.

No correctness, security, or performance concerns.

@MauriceVanVeen MauriceVanVeen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@scottf
scottf merged commit 7d69e18 into main Sep 3, 2026
8 checks passed
@scottf
scottf deleted the 2-15-max-consumers branch September 3, 2026 14:33
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.

3 participants