Skip to content

Fix ConfigureTestConfiguration being invoked twice#5203

Merged
thomhurst merged 1 commit intomainfrom
fix/configure-test-configuration-called-twice
Mar 21, 2026
Merged

Fix ConfigureTestConfiguration being invoked twice#5203
thomhurst merged 1 commit intomainfrom
fix/configure-test-configuration-called-twice

Conversation

@thomhurst
Copy link
Owner

Summary

Fixes #5195

  • ConfigureTestConfiguration was passed to GetIsolatedFactory twice: once as startup configuration (applied via UseSetting before Program.cs) and once as app configuration (applied via ConfigureAppConfiguration after Program.cs). This caused the method to run twice — once before and once after Factory.ConfigureWebHost — breaking the documented lifecycle order.
  • Removed the redundant configureIsolatedStartupConfiguration parameter from GetIsolatedFactory so ConfigureTestConfiguration only runs once, at the correct point in the lifecycle (after factory configuration, allowing test overrides).
  • Updated lifecycle documentation and tests to reflect the corrected execution order.

Test plan

  • Verify TUnit.Example.Asp.Net.TestProject lifecycle order tests pass (especially FactoryMethodOrderTests.Full_Relative_Order)
  • Verify ConfigureTestConfiguration is only invoked once per test
  • Verify test configuration values properly override factory configuration values

ConfigureTestConfiguration was passed to GetIsolatedFactory both as
startup configuration (applied via UseSetting before Program.cs) and
as app configuration (applied via ConfigureAppConfiguration after
Program.cs), causing it to run twice — once before and once after
Factory.ConfigureWebHost.

Remove the redundant startup configuration parameter so
ConfigureTestConfiguration only runs once, at the correct point in
the lifecycle (after factory configuration, allowing test overrides).
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review

Summary

This is a clean, well-scoped fix for a genuine double-invocation bug in the lifecycle. The root cause is clear, the fix is surgical, and the documentation + tests are updated in lockstep.


The Core Fix

The bug in TestWebApplicationFactory.cs was straightforward: ConfigureTestConfiguration was passed twice to GetIsolatedFactory:

  1. As configureIsolatedStartupConfiguration → applied via UseSetting (pre-startup, before Program.cs)
  2. As configureIsolatedAppConfiguration → applied via ConfigureAppConfiguration (post-factory, at app-startup time)

Removing the first pass is the right call. The semantically correct place for test overrides is as ConfigureAppConfiguration, after the factory has had its chance to configure the host.


Execution Order Clarification (No Action Required — Just Worth Knowing)

The new documented order (5 = ConfigureWebHostBuilder, 6 = ConfigureTestConfiguration) maps to two different phases of execution:

  • configureWebHostBuilder?.Invoke(builder) is called synchronously at the end of the WithWebHostBuilder lambda (factory-setup time)
  • ConfigureAppConfiguration callbacks, including configureIsolatedAppConfiguration, run at app-startup time

This means ConfigureWebHostBuilder code runs at factory-construction time, while ConfigureTestConfiguration runs during application startup — which is precisely what makes test config able to override factory config. The ordering is correct and the docs now accurately reflect this.

One subtle implication: if a user calls builder.ConfigureAppConfiguration(...) inside their ConfigureWebHostBuilder override, that registration would be added after configureIsolatedAppConfiguration was registered — so it would execute after ConfigureTestConfiguration at app-startup time. This is likely the intended "escape hatch" behavior (last word goes to the escape hatch), but it's worth a comment in the code or docs if users ever encounter confusion here.


TestsBase.cs Cleanup

Removing the if (ConfigureTestConfigurationCalledOrder == 0) guard is exactly right — it was a workaround for a bug that no longer exists. Cleaning it up avoids misleading future readers about why the guard was there.


Minor Observation

The configureWebHostBuilder?.Invoke(builder) call is at the end of the WithWebHostBuilder lambda, after ConfigureAppConfiguration and ConfigureTestServices registrations. This is intentional and correct — it ensures that by the time user code runs in ConfigureWebHostBuilder, all the standard registrations are already in place. This is good design; just noting it's load-bearing.


Verdict

LGTM. The fix is minimal, correct, and properly validated. The documentation and test updates are accurate and helpful. No architectural concerns.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review

Summary

This is a clean, well-scoped fix for a genuine double-invocation bug in the ConfigureTestConfiguration lifecycle. The root cause is clear, the fix is surgical, and the documentation + tests are updated in lockstep.


The Core Fix

The bug in TestWebApplicationFactory.cs was straightforward: ConfigureTestConfiguration was passed twice to GetIsolatedFactory:

  1. As configureIsolatedStartupConfiguration applied via UseSetting (pre-startup, before Program.cs)
  2. As configureIsolatedAppConfiguration applied via ConfigureAppConfiguration (post-factory, at app-startup time)

Removing the first pass is the right call. The semantically correct place for test overrides is as ConfigureAppConfiguration, after the factory has had its chance to configure the host.


Execution Order Clarification (No Action Required, Worth Knowing)

The new documented order (5 = ConfigureWebHostBuilder, 6 = ConfigureTestConfiguration) maps to two different phases of execution:

  • configureWebHostBuilder?.Invoke(builder) is called synchronously at the end of the WithWebHostBuilder lambda (factory-setup time)
  • ConfigureAppConfiguration callbacks, including configureIsolatedAppConfiguration, run at app-startup time

This means ConfigureWebHostBuilder code runs at factory-construction time, while ConfigureTestConfiguration runs during application startup, which is precisely what makes test config able to override factory config. The ordering is correct and the docs now accurately reflect this.

One subtle implication: if a user calls builder.ConfigureAppConfiguration(...) inside their ConfigureWebHostBuilder override, that registration is added after configureIsolatedAppConfiguration was registered, so it would execute after ConfigureTestConfiguration at app-startup time. This is likely the intended "escape hatch" behavior (last word goes to the escape hatch), but it may be worth a doc note if users ever encounter confusion here.


TestsBase.cs Cleanup

Removing the if (ConfigureTestConfigurationCalledOrder == 0) guard is exactly right. It was a workaround for a bug that no longer exists. Leaving it would mislead future readers into thinking there is still an expected double-invocation.


Verdict

LGTM. The fix is minimal, correct, and properly validated. The documentation and test updates are accurate and consistent with the actual execution model. No architectural concerns.

@thomhurst thomhurst enabled auto-merge (squash) March 21, 2026 13:13
@thomhurst thomhurst merged commit bfc204c into main Mar 21, 2026
15 of 16 checks passed
@thomhurst thomhurst deleted the fix/configure-test-configuration-called-twice branch March 21, 2026 13:28
This was referenced Mar 21, 2026
This was referenced Mar 23, 2026
github-actions bot pushed a commit to IntelliTect/CodingGuidelines that referenced this pull request Mar 23, 2026
[//]: # (dependabot-start)
⚠️  **Dependabot is rebasing this PR** ⚠️ 

Rebasing might not happen immediately, so don't worry if this takes some
time.

Note: if you make any changes to this PR yourself, they will take
precedence over the rebase.

---

[//]: # (dependabot-end)

Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.19.57 to
1.21.6.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit.Core's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.21.6

<!-- Release notes generated using configuration in .github/release.yml
at v1.21.6 -->

## What's Changed
### Other Changes
* perf: replace object locks with Lock type for efficient
synchronization by @​thomhurst in
thomhurst/TUnit#5219
* perf: parallelize test metadata collection for source-generated tests
by @​thomhurst in thomhurst/TUnit#5221
* perf: use GetOrAdd args overload to eliminate closure allocations in
event receivers by @​thomhurst in
thomhurst/TUnit#5222
* perf: self-contained TestEntry<T> with consolidated switch invokers
eliminates per-test JIT by @​thomhurst in
thomhurst/TUnit#5223
### Dependencies
* chore(deps): update tunit to 1.21.0 by @​thomhurst in
thomhurst/TUnit#5220


**Full Changelog**:
thomhurst/TUnit@v1.21.0...v1.21.6

## 1.21.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.21.0 -->

## What's Changed
### Other Changes
* perf: reduce ConcurrentDictionary closure allocations in hot paths by
@​thomhurst in thomhurst/TUnit#5210
* perf: reduce async state machine overhead in test execution pipeline
by @​thomhurst in thomhurst/TUnit#5214
* perf: reduce allocations in EventReceiverOrchestrator and
TestContextExtensions by @​thomhurst in
thomhurst/TUnit#5212
* perf: skip timeout machinery when no timeout configured by @​thomhurst
in thomhurst/TUnit#5211
* perf: reduce allocations and lock contention in ObjectTracker by
@​thomhurst in thomhurst/TUnit#5213
* Feat/numeric tolerance by @​agray in
thomhurst/TUnit#5110
* perf: remove unnecessary lock in ObjectTracker.TrackObjects by
@​thomhurst in thomhurst/TUnit#5217
* perf: eliminate async state machine in
TestCoordinator.ExecuteTestAsync by @​thomhurst in
thomhurst/TUnit#5216
* perf: eliminate LINQ allocation in ObjectTracker.UntrackObjectsAsync
by @​thomhurst in thomhurst/TUnit#5215
* perf: consolidate module initializers into single .cctor via partial
class by @​thomhurst in thomhurst/TUnit#5218
### Dependencies
* chore(deps): update tunit to 1.20.0 by @​thomhurst in
thomhurst/TUnit#5205
* chore(deps): update dependency nunit3testadapter to 6.2.0 by
@​thomhurst in thomhurst/TUnit#5206
* chore(deps): update dependency cliwrap to 3.10.1 by @​thomhurst in
thomhurst/TUnit#5207


**Full Changelog**:
thomhurst/TUnit@v1.20.0...v1.21.0

## 1.20.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.20.0 -->

## What's Changed
### Other Changes
* Fix inverted colors in HTML report ring chart due to locale-dependent
decimal formatting by @​Copilot in
thomhurst/TUnit#5185
* Fix nullable warnings when using Member() on nullable properties by
@​Copilot in thomhurst/TUnit#5191
* Add CS8629 suppression and member access expression matching to
IsNotNullAssertionSuppressor by @​Copilot in
thomhurst/TUnit#5201
* feat: add ConfigureAppHost hook to AspireFixture by @​thomhurst in
thomhurst/TUnit#5202
* Fix ConfigureTestConfiguration being invoked twice by @​thomhurst in
thomhurst/TUnit#5203
* Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T> by
@​thomhurst in thomhurst/TUnit#5204
### Dependencies
* chore(deps): update dependency gitversion.tool to v6.6.2 by
@​thomhurst in thomhurst/TUnit#5181
* chore(deps): update dependency gitversion.msbuild to 6.6.2 by
@​thomhurst in thomhurst/TUnit#5180
* chore(deps): update tunit to 1.19.74 by @​thomhurst in
thomhurst/TUnit#5179
* chore(deps): update verify to 31.13.3 by @​thomhurst in
thomhurst/TUnit#5182
* chore(deps): update verify to 31.13.5 by @​thomhurst in
thomhurst/TUnit#5183
* chore(deps): update aspire to 13.1.3 by @​thomhurst in
thomhurst/TUnit#5189
* chore(deps): update dependency stackexchange.redis to 2.12.4 by
@​thomhurst in thomhurst/TUnit#5193
* chore(deps): update microsoft/setup-msbuild action to v3 by
@​thomhurst in thomhurst/TUnit#5197


**Full Changelog**:
thomhurst/TUnit@v1.19.74...v1.20.0

## 1.19.74

<!-- Release notes generated using configuration in .github/release.yml
at v1.19.74 -->

## What's Changed
### Other Changes
* feat: per-hook activity spans with method names by @​thomhurst in
thomhurst/TUnit#5159
* fix: add tooltip to truncated span names in HTML report by @​thomhurst
in thomhurst/TUnit#5164
* Use enum names instead of numeric values in test display names by
@​Copilot in thomhurst/TUnit#5178
* fix: resolve CS8920 when mocking interfaces whose members return
static-abstract interfaces by @​lucaxchaves in
thomhurst/TUnit#5154
### Dependencies
* chore(deps): update tunit to 1.19.57 by @​thomhurst in
thomhurst/TUnit#5157
* chore(deps): update dependency gitversion.msbuild to 6.6.1 by
@​thomhurst in thomhurst/TUnit#5160
* chore(deps): update dependency gitversion.tool to v6.6.1 by
@​thomhurst in thomhurst/TUnit#5161
* chore(deps): update dependency polyfill to 9.20.0 by @​thomhurst in
thomhurst/TUnit#5163
* chore(deps): update dependency polyfill to 9.20.0 by @​thomhurst in
thomhurst/TUnit#5162
* chore(deps): update dependency polyfill to 9.21.0 by @​thomhurst in
thomhurst/TUnit#5166
* chore(deps): update dependency polyfill to 9.21.0 by @​thomhurst in
thomhurst/TUnit#5167
* chore(deps): update dependency polyfill to 9.22.0 by @​thomhurst in
thomhurst/TUnit#5168
* chore(deps): update dependency polyfill to 9.22.0 by @​thomhurst in
thomhurst/TUnit#5169
* chore(deps): update dependency coverlet.collector to 8.0.1 by
@​thomhurst in thomhurst/TUnit#5177

## New Contributors
* @​lucaxchaves made their first contribution in
thomhurst/TUnit#5154

**Full Changelog**:
thomhurst/TUnit@v1.19.57...v1.19.74

Commits viewable in [compare
view](thomhurst/TUnit@v1.19.57...v1.21.6).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TUnit.Core&package-manager=nuget&previous-version=1.19.57&new-version=1.21.6)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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.

[Bug]: Incorrect integration test lifecycle order

1 participant