Skip to content

Fix the GH-4516 negative control that is failing CI on every PR - #4560

Merged
jeremydmiller merged 1 commit into
mainfrom
fix-4516-negative-control-ci
Sep 23, 2026
Merged

jeremydmiller merged 1 commit into
mainfrom
fix-4516-negative-control-ci

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

This is what is making #4553, #4557 and #4558 all fail. All three fail on exactly one job (test, ~5 min) with the same assertion, two of them from contributors. It is my regression from #4551, live on main since yesterday.

[FAILED] Wolverine.Http.Tests.unknown_tenant_problem_details_4516.without_the_opt_in_the_unknown_tenant_still_escapes
  Shouldly.ShouldAssertException : Task `async () => await host.Scenario(...)`
      should throw JasperFx.MultiTenancy.UnknownTenantIdException
      but threw Alba.ScenarioAssertionException

What was wrong

The test was self-contradictory. It asked Alba to assert a 404 inside a scenario it expected to throw:

await Should.ThrowAsync<UnknownTenantIdException>(async () =>
    await host.Scenario(x =>
    {
        x.Get.Url("/gh4516/tenanted?tenantId=ghost");
        x.StatusCodeShouldBe(404);      // <- asserted on a request expected to blow up
    }));

Locally the UnknownTenantIdException propagated before Alba evaluated its assertions, so the expected exception won and it passed. On CI the host turns the exception into a 500 first, so Alba's own 404 assertion fires and raises ScenarioAssertionException — a different type, so Should.ThrowAsync fails.

The fix

The control's purpose is narrower than what it asserted: it exists to show that MapUnknownTenantToNotFound() is what produces the 404, not something else in the pipeline. It is not a claim about how an unmapped failure surfaces — and that is precisely the part that varies by environment.

It now uses IgnoreStatusCode() and asserts only that the response is not a mapped 404, tolerating the other surfacing (the exception escaping the scenario) and nothing else — an unexpected exception type still fails.

On my part in this

The local suite was green when I merged #4551, so I had no signal. But I wrote an assertion that could only hold under one of two environment-dependent behaviours and didn't notice I'd coupled the control to the wrong thing. Worth flagging given it blocked two contributors' PRs overnight.

Wolverine.Http.Tests  unknown_tenant_problem_details_4516  5/5

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

`unknown_tenant_problem_details_4516.without_the_opt_in_the_unknown_tenant_still_escapes`
went red on CI the moment #4551 merged, and it takes the whole `test` job with it -- so
every open PR is red regardless of its contents (#4553, #4557, #4558 at the time of
writing, two of them from contributors).

The test was self contradictory. It asked Alba to assert a 404 inside a scenario it
expected to THROW:

    await Should.ThrowAsync<UnknownTenantIdException>(async () =>
        await host.Scenario(x =>
        {
            x.Get.Url("/gh4516/tenanted?tenantId=ghost");
            x.StatusCodeShouldBe(404);      // <- asserted on a request expected to blow up
        }));

Locally the UnknownTenantIdException propagated before Alba evaluated its assertions, so
the expected exception won and the test passed. On CI the host turned the exception into
a 500 first, so Alba's own 404 assertion fired and raised ScenarioAssertionException
instead of UnknownTenantIdException -- a different type, so Should.ThrowAsync failed.

The control's actual purpose is narrower than what it asserted: it exists to show that
MapUnknownTenantToNotFound() is what produces the 404, not something else in the pipeline.
It is not a claim about HOW an unmapped failure surfaces, and that is exactly the part
that varies by environment. It now uses IgnoreStatusCode() and asserts only that the
response is not a mapped 404, tolerating the other surfacing -- the exception escaping the
scenario -- and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
@jeremydmiller
jeremydmiller merged commit 97d2caa into main Sep 23, 2026
44 checks passed
jeremydmiller added a commit that referenced this pull request Sep 23, 2026
jeremydmiller added a commit to erdtsieck/wolverine that referenced this pull request Sep 23, 2026
jeremydmiller added a commit to Trasvi/wolverine that referenced this pull request Sep 23, 2026
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