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

InMemory DbContext does not respect CancellationToken on SaveChangesAsync #13368

Closed
MatthewLymer opened this issue Sep 19, 2018 · 8 comments
Closed
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported

Comments

@MatthewLymer
Copy link

MatthewLymer commented Sep 19, 2018

When using an InMemory DbContext, the SaveChangesAsync does not respect the cancellation token being passed in.

Steps to reproduce

  1. Create a DbContext called TenantDbContext
  2. Create a type called Setting, expose a DbSet<Settings>
  3. Run the following unit test:
	[Fact]
	public async Task ShouldPersistResultIfScrapeTimesOut()
	{
		var options = new DbContextOptionsBuilder<TenantDbContext>()
			.UseInMemoryDatabase(Guid.NewGuid().ToString())
			.Options;

		using (var dbContext = new TenantDbContext(options))
		{
			var setting = new Setting
			{
				Name = "Foo", 
				Value = "Bar"
			};
			
			dbContext.Settings.Add(setting);
			
			await Assert.ThrowsAnyAsync<OperationCanceledException>(
				() => dbContext.SaveChangesAsync(new CancellationToken(true)));
		}

		using (var dbContext = new TenantDbContext(options))
		{
			var settings = await dbContext.Settings.ToListAsync(
				CancellationToken.None);
			
			Assert.Empty(settings);
		}
	}

Further technical details

EF Core version

  • Microsoft.EntityFrameworkCore.Design v2.1.0
  • Microsoft.EntityFrameworkCore.InMemory v2.1.1

Database Provider

  • Microsoft.EntityFrameworkCore.InMemory

Operating system

  • Windows 10

IDE

  • Visual Studio 2017 15.8.0
@MatthewLymer
Copy link
Author

MatthewLymer commented Sep 19, 2018

My current workaround for getting my unit tests to behave as expected.

	public override Task<int> SaveChangesAsync(
		CancellationToken cancellationToken = new CancellationToken())
	{
		if (Database.IsInMemory())
		{
			cancellationToken.ThrowIfCancellationRequested();
		}
		
		return base.SaveChangesAsync(cancellationToken);
	}

@pmiddleton
Copy link
Contributor

@MatthewLymer

The InMemory provider does not make any async calls and therefore does not use the cancellationToken. Internally it it just performs a Task.FromResult inside of SaveChangesAsync

@MatthewLymer
Copy link
Author

I assumed as much, however I assume the entire purpose of the InMemory database is to facilitate testing, so I think having behavior closely match other providers would be desirable.

@hannahchan
Copy link

hannahchan commented Apr 15, 2020

So it's been a number of years since this was reported and I thought I should mention that I've just run into the same issue. I'm trying to unit test my repository class for cooperative cancellation by making sure it's passing through the CancellationToken to the DbContext.

@smitpatel
Copy link
Member

See #18457

@ajcvickers
Copy link
Member

We recommend against using the in-memory provider for testing--see Testing EF Core Applications. While we have no plans to remove the in-memory provider, we will not be adding any new features to this provider because we believe valuable development time is better spent in other areas. When feasible, we plan to still fix regressions in existing behavior.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 26, 2022
@ajcvickers ajcvickers removed this from the Backlog milestone Oct 26, 2022
@slaneyrw
Copy link

slaneyrw commented Mar 2, 2023

@ajcvickers

Just like to add that you cannot just replace the InMemory database with SQLite as a testing replacement for a SQL Server based context. Whilst new features are available ( Tx ), some are removed... like Concurrency ( timestamp/rowversion ) support. The suggested "solution" involves modifying the real code under test if running SQLite - not an acceptable approach IMHO. If the InMemory is removed, EF based solution may become untestable in some circumstances.

Please respect the cancellationToken in the InMemory provider... it's a real simple check. It avoids us from having to insert "ThrowIfCancellationRequested()" everywhere our code because we want to test exception handling.

The debate about a suitable test DB engine replacement for Unit tests is not a simple one, but all DB providers MUST respect cancellation tokens in all async methods.

@roji
Copy link
Member

roji commented Mar 2, 2023

The debate about a suitable test DB engine replacement for Unit tests is not a simple one, but all DB providers MUST respect cancellation tokens in all async methods.

FWIW this isn't quite true; cancellation in general is a best-effort, race-sensitive mechanism - there's never any 100% guarantee that cancellation will actually work, and your code and tests generally shouldn't rely on that. It's true that in many cases, calling a method with a cancelled token immediately causes an OperationCanceledException to be thrown, but this isn't a hard rule; for example, if the method doesn't need to complete any I/O (e.g. since data isn't buffered in memory), then it's reasonable for it to just complete rather than throw.

Note that I'm not saying that InMemory necessarily shouldn't throw when passed a cancelled token, but it's just another example of why using InMemory for testing isn't a good choice. If you want to test that your application code actually passes the cancellation token to EF's SaveChangesAsync, I'd consider mocking DbContext and asserting via the mock that the token was received.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported
Projects
None yet
Development

No branches or pull requests

8 participants