Make Cosmos provider throw for sync I/O #33386
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #32563 This change adds an error via logging which is thrown on any use of sync I/O with Cosmos. This can currently be disabled to allow sync again, but ultimately this will be removed--probably in 11. Also, makes all test infrastructure async-only. This means an EF provider only needs to support async APIs to use our test infra.
cincuranet
approved these changes
Mar 25, 2024
roji
reviewed
Aug 24, 2024
| @@ -43,7 +43,7 @@ public virtual void Values_arent_compared_by_reference() | |||
| } | |||
|
|
|||
| [ConditionalFact] | |||
| public virtual void Mutation_of_tracked_values_does_not_mutate_values_in_store() | |||
| public virtual async void Mutation_of_tracked_values_does_not_mutate_values_in_store() | |||
There was a problem hiding this comment.
@ajcvickers this means that the test cannot be overridden to e.g. catch an exception (in PG...); I'm actually not too sure how xunit can even run async void tests... Was there a specific reason to use async void here?
There was a problem hiding this comment.
Async void tests are bad! They are one of things that cause test flakiness.
With regard to overriding, I typically just skip the sync path and only test the async if an exception is expected. There are a lot of examples of this.
There was a problem hiding this comment.
xUnit analyzers don't catch this because they don't run for derived attributes: xunit/xunit#1963
Filed #34536
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #32563
This change adds an error via logging which is thrown on any use of sync I/O with Cosmos. This can currently be disabled to allow sync again, but ultimately this will be removed--probably in 11.
Also, makes all test infrastructure async-only. This means an EF provider only needs to support async APIs to use our test infra.