Getting Error in ResetAsync with EmptyDatabase
Closed this issue · 11 comments
Hi,
When ResetAsync
call empty database, I get this error:
System.InvalidOperationException: CommandText property has not been initialized
I think this because, graph doesn't find any table to delete (DeleteSql is empty), but in this case spawner should not throw an exception, and it should skip it because there is no work to do.
Why would you run Respawn on an empty database? That seems like I'd have screwed something up.
Maybe it not uses very much, but if Respawner run on empty database it throws an exception. We can prevent this.
Why shouldn't it be an exception? What is a testing scenario in which I'd run Respawn against an empty database?
You're right, exception is better. but I think exception message is not very clear. Maybe it should throw an appropriate exception like database is empty
System.InvalidOperationException: CommandText property has not been initialized
Yeah that's a bad exception message for sure. I think it's worth a discussion on what the behavior should be, a description exception or do nothing. I could see both options being correct.
Yes, Both options work fine :) I sent I pull request for doing nothing without exception.
You can choose which one is better fit.
Thanks for this great library.
Do you have a plan for supporting Mongo?
Re: Mongo - no, I personally use Mongo2Go which works fine for me. I'd just create a separate library anyway, since the mechanism would be completely different.
Been pulling my hair out over this little guy off and on as I've had time to dig into it over the last couple of months. I think where this is primarily biting me in the ass is only on the initial ResetAsync when integration tests begin to run for the very first time. Example of this sort of setup with xUnit.
This is currently using 5.0.1 because moving to 6.0.0 causes the aforementioned issue. I narrowed it down to this particular line in 5.0.1 -
452bacf#diff-c4cbce0556ef97e7f50361dfcd87eb425b7693f4f97d870fddd447d9b410ac13L38
Which matches the behavior of what @mehdihadeli has submitted in his PR (and what is now missing) albeit in a slightly different location.
The new location seems appropriate considering the refactoring toward CreateAsync
which now builds the delete tables initially.
Love this library & thanks so much for both this, AutoMapper, and Mediatr!
Since the delete SQL is cached, what should the behavior be? In your collection fixture that value would be cached as empty.
I thought about it and I actually like an exception. Not the current one, of course, but I’d be even more confused (and in my angry bias, blame the library 😅) if it just silently deleted nothing and my database showed tables with data in them from multiple tests because my setup was bad. My 2c.
Yeah since it caches the value, you don't have any feedback that it's not clearing the database later. I'd opt for a better exception message too.