Skip to content

Tests: seven cache and HTTP behaviours the unit tests don't pin down #1035

Open

Description

@Trenyx

What is the problem your feature solves, or the need it fulfills?

We ran a planted-bug exercise on Pingora at 4487f7b: in a private copy, we made one small, realistic change at a time and ran the existing tests. These seven changes left every test of pingora-cache, pingora-http and pingora-header-serde passing:

  1. pingora-cache/src/filters.rs: revalidation sends the stale asset's Last-Modified as If-Unmodified-Since instead of If-Modified-Since, so the origin never answers 304.
  2. pingora-cache/src/variance.rs: the Vary variance key hashes only header names, not values, so Accept-Encoding: gzip and br share one cached variant.
  3. pingora-cache/src/meta.rs: the serve-stale windows become end-exclusive, so at exactly the window's end the stale asset can no longer be served.
  4. pingora-cache/src/lib.rs: a writer that finds the response uncacheable releases the cache lock as a transient error instead of giving up, so waiting readers re-elect writers one at a time.
  5. pingora-http/src/authority.rs: the request-target scheme check becomes case-sensitive, so HTTP:// targets skip the ambiguous-authority rules.
  6. pingora-http/src/lib.rs: remove_header leaves a stale entry in the case-preserving name map.
  7. pingora-header-serde/src/lib.rs: deserializing cached headers inserts instead of appending, so repeated fields such as Set-Cookie collapse to the last one.

None of these bugs is in Pingora today. They are regressions the current tests would let through.

Describe the solution you'd like

One unit test per behaviour, in the existing test module of each file. Tests only: no change to library code, no new dependencies. A PR referencing this issue follows.

Additional context

We ran the tests of those crates (plus pingora-load-balancing, pingora-ketama, pingora-limits and pingora-pool), not pingora-proxy's integration tests, which need OpenResty. If one of those already covers a case, I'll drop that test from the PR.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions