marten icon indicating copy to clipboard operation
marten copied to clipboard

Asserting exceptions are actually thrown fails tests in UniqueIndexTests.cs

Open Shield1739 opened this issue 7 months ago • 3 comments

Asserting that exceptions are actually thrown makes the 2 inline tests fail.

SaveChanges isn't actually throwing any type of exception.

Shield1739 avatar Nov 10 '23 18:11 Shield1739

@Shield1739 with regards the changes which you have in this PR i.e. existing code and the updated are all equivalent,

try
{
    session.SaveChanges();
 }
catch (DocumentAlreadyExistsException exception)
{
    ((PostgresException)exception.InnerException)?.SqlState.ShouldBe(UniqueSqlState);
}

to

var exception = Should.Throw<DocumentAlreadyExistsException>(() => session.SaveChanges());
((PostgresException)exception.InnerException)?.SqlState.ShouldBe(UniqueSqlState);

If SaveChanges is not throwing exception, it won't get into the catch block hence why should the test fail?

I am not sure what issue you are attempting to highlight. Could you please elaborate?

mysticmind avatar Nov 17 '23 13:11 mysticmind

@mysticmind

By the name of the test

"given_two_events_with_the_same_value_for_unique_field_with_single_property_when_inline_transformation_is_applied_then_throws_exception"

I understood that it is meant to throw an exception. Isn't it? It's appending two events with the same name and email so the unique index fails and the documents are not created, but the SaveChanges does not throw an exception.

I noticed this when i was adding the name of the failed constraint to the exception that marten throws out to be able to easily identify why a document was not inserted. I noticed that when the events are appended the document creation does fails, but the exception isn't actually thrown.

Shouldn't it throw an exception with SaveChanges when document creation fails? Or is this meant to be the expected behavior?

Shield1739 avatar Nov 18 '23 08:11 Shield1739

@Shield1739 Thanks for the details and explanation, will check further and keep you posted.

mysticmind avatar Nov 22 '23 04:11 mysticmind