Skip to content

fix: extend transaction_get lookup to observe all pending cache obligations#627

Merged
THad04 merged 4 commits into
mainfrom
feature/cache-test-collapse-626
May 29, 2026
Merged

fix: extend transaction_get lookup to observe all pending cache obligations#627
THad04 merged 4 commits into
mainfrom
feature/cache-test-collapse-626

Conversation

@THad04
Copy link
Copy Markdown
Contributor

@THad04 THad04 commented May 27, 2026

Closes #626

Description

This PR addresses issue #626. The goal was to implement an integration test verifying request-collapsing safety when handling dynamic HTTP Vary header evaluations. The test passed, showing that this edge case is covered.

Changes

  1. **Deterministic Test: Added test_collapse_from_later_vary to test-fixtures/src/bin/cache.rs. It handles explicit host-side handle cleanup via .cancel_insert_or_update().

Verification

  • Verified that all 160 integration tests and 112 unit tests pass 100% green

@THad04 THad04 requested a review from cceckman-at-fastly May 27, 2026 19:56
@THad04 THad04 self-assigned this May 27, 2026
Copy link
Copy Markdown
Contributor

@cceckman-at-fastly cceckman-at-fastly left a comment

Choose a reason for hiding this comment

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

Per chat -- I'm actually confused as to how the test passes at HEAD / why the fix fixes it; it looks to me like the fix would collapse when we shouldn't? But I suspect I'm missing something

Comment thread test-fixtures/src/bin/cache.rs Outdated
Comment thread test-fixtures/src/bin/cache.rs Outdated
@THad04 THad04 force-pushed the feature/cache-test-collapse-626 branch from 2e24b2f to 5559cb3 Compare May 28, 2026 20:08
Comment thread test-fixtures/src/bin/cache.rs Outdated
Comment thread test-fixtures/src/bin/cache.rs Outdated
Comment thread test-fixtures/src/bin/cache.rs Outdated
Copy link
Copy Markdown
Contributor

@cceckman-at-fastly cceckman-at-fastly left a comment

Choose a reason for hiding this comment

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

Overall looks good! A couple cleanup suggestions & one point for discussion, but overall this seems like it does the trick!

@cceckman-at-fastly cceckman-at-fastly added the skip-changelog This change does not require a changelog entry. label May 29, 2026
@THad04 THad04 merged commit 627ed46 into main May 29, 2026
14 of 15 checks passed
@THad04 THad04 deleted the feature/cache-test-collapse-626 branch May 29, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog This change does not require a changelog entry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache: test collapse from later Vary

2 participants