Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation test refactoring initial work #35652

Merged
merged 1 commit into from
Feb 28, 2025
Merged

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Feb 19, 2025

Motivation:
We want to be able to re-use existing navigation tests (currently spread around multiple test classes - complex navs, gears of war, northwind, json)
for the upcoming optional complex types, complex collections and json-mapped complex types (all highly requested features in EF).
We also want to be able to increase coverage for existing features, e.g. we have very good coverage of navigations in json test suite, but it's not used for regular or owned entities.
This work can also be used by provider writers (e.g. Mongo) to boost their coverage.

Details:

  • using common model for entity, owned, json and complex type navigations
  • 4 levels: root, trunk, branch, leaf
  • optional reference, required reference (dependent to principal), collection
  • for now just testing projection scenarios as proof of concept (tracking / notracking)

TODO:

  • fix owned sqlite model,
  • add model with inheritance,
  • move actual tests from existing test suites,
  • add migration check (that model can be migrated to from scratch and that noop is actual noop),
  • implement InMemory tests.

@maumar maumar requested a review from Copilot February 19, 2025 00:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.

@maumar maumar force-pushed the navigations_test_refactoring branch 2 times, most recently from 621497e to 8d1a4d6 Compare February 19, 2025 01:04
@maumar maumar requested a review from a team February 19, 2025 01:06
@maumar maumar force-pushed the navigations_test_refactoring branch 3 times, most recently from 64b65bd to f42c629 Compare February 24, 2025 21:22
@AndriySvyryd
Copy link
Member

Are you planning to replace the commented-out tests in this PR or a separate one?

@maumar
Copy link
Contributor Author

maumar commented Feb 25, 2025

@AndriySvyryd this PR, I will add some more tests and do the clean-up of the commented ones. But you can take a look at the model and the test class structure.

@maumar maumar force-pushed the navigations_test_refactoring branch 2 times, most recently from 16601a6 to fac79bf Compare February 28, 2025 03:15
We want to be able to re-use existing navigation tests (currently spread around multiple test classes - complex navs, gears of war, northwind, json)
for the upcoming optional complex types, complex collections and json-mapped complex types (all highly requested features in EF).
We also want to be able to increase coverage for existing features, e.g. we have very good coverage of navigations in json test suite, but it's not used for regular or owned entities.
This work can also be used by provider writers (e.g. Mongo) to boost their coverage.

Navigation test refactoring:
- using common model for entity, owned, json and complex type navigations
- 4 levels: root, trunk, branch, leaf
- optional reference, required reference (dependent to principal), collection
- for now just testing projection scenarios as proof of concept (tracking / notracking)

TODO:
- fix owned sqlite model,
- add model with inheritance,
- move actual tests from existing test suites (ongoing),
- add migration check (that model can be migrated to from scratch and that noop is actual noop),
- implement InMemory tests.

- added include stub
- added cosmos tests
@maumar maumar force-pushed the navigations_test_refactoring branch from fac79bf to e1cc089 Compare February 28, 2025 05:25
@maumar maumar merged commit 9c020be into main Feb 28, 2025
7 checks passed
@maumar maumar deleted the navigations_test_refactoring branch February 28, 2025 06:29

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_trunk_collection(bool async)
Copy link
Member

Choose a reason for hiding this comment

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

@maumar remember that as discussed, I think we want to keep tests which rely on collection relationships separate from those which only rely on reference relationships - this is because we have non-JSON complex/owned mapping (table splitting), which will only be supported with reference relationships.

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.

3 participants