Skip to content

Throw on EF anti-patterns in tests and fix the ones found - #1421

Merged
SimonCropp merged 1 commit into
mainfrom
verify-ef-anti-patterns
Sep 28, 2026
Merged

SimonCropp merged 1 commit into
mainfrom
verify-ef-anti-patterns

Conversation

@SimonCropp

Copy link
Copy Markdown
Owner

Adds Verify.EntityFramework (16.0.0-beta.4) to the tests and enables ThrowOnAntiPatterns for every test SqlInstance, then fixes what it found in the library.

Fixes

  • Connection count. The count ran on the page query, so it carried ordering, includes, split query and tracking options, all of which a count ignores. They are now stripped from the query's own chain before counting. Stripping stops at any Skip/Take, since below one the ordering decides which rows are counted.
  • orderBy argument. It was applied as a second OrderBy after any ordering the resolver applied, and EF discards the first. The resolver's ordering is now removed before the argument's ordering is applied.

Test changes

  • New OrderByArgumentReplacesResolverOrdering test, over a new orderedParentEntities field whose resolver orders descending. It fails without the orderBy fix.
  • Connection_without_first_or_last_returns_everything: the join to ParentEntities forced by the discarded OrderBy(_ => _.Parent) is gone from the SQL.
  • SchemaPrint: includes the new field.

Add Verify.EntityFramework and enable ThrowOnAntiPatterns for every test SqlInstance.

Fixes for what it found:

- Connection count: the count ran on the page query, so it carried ordering, includes,
  split query and tracking options, all of which a count ignores. They are now stripped
  from the query's own chain before counting, stopping at any Skip or Take.
- orderBy argument: it was applied as a second OrderBy after any ordering the resolver
  applied, which EF discards. The resolver's ordering is now removed first. This also
  drops a join the discarded ordering forced in
  Connection_without_first_or_last_returns_everything.

Add OrderByArgumentReplacesResolverOrdering, over a new orderedParentEntities field, to
cover the orderBy case for list fields.
@SimonCropp SimonCropp added this to the 35.3.2 milestone Sep 28, 2026
@SimonCropp
SimonCropp merged commit 9178ad7 into main Sep 28, 2026
5 checks passed
@SimonCropp
SimonCropp deleted the verify-ef-anti-patterns branch September 28, 2026 06:17
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