Skip to content

Conversation

@Shield1739
Copy link

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

SaveChanges isn't actually throwing any type of exception.

@Shield1739 Shield1739 changed the title Assert exceptions are actually thrown in UniqueIndexTests.cs Asserting exceptions are actually thrown fails tests in UniqueIndexTests.cs Nov 10, 2023
@mysticmind
Copy link
Member

mysticmind commented Nov 17, 2023

@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?

@Shield1739
Copy link
Author

Shield1739 commented Nov 18, 2023

@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?

@mysticmind
Copy link
Member

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

@oskardudycz oskardudycz force-pushed the assert_exception_unique_index_test branch from 973ded0 to 426a241 Compare November 22, 2023 10:10
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.

2 participants