CSHARP-5589: Improve BSON project test coverage #1694
Conversation
BorisDog
changed the title
BorisDog
changed the title
| public class BsonExceptionTests | ||
| { | ||
| [Fact] | ||
| public void constructor_with_format_and_args_should_format_message_correctly() |
There was a problem hiding this comment.
Should we add test to check mismatching format string and arguments list?
Like:
var exception = new BsonException("Error code: {0}, message: {1}", 123);
| } | ||
|
|
||
| [Fact] | ||
| public void constructor_with_format_and_args_should_handle_empty_args() |
There was a problem hiding this comment.
Test method name is wrong, because it actually uses another constructor: the one accepting a single string as a parameter.
|
|
||
| namespace MongoDB.Bson.Tests.Exceptions | ||
| { | ||
| public class BsonSerializationExceptionTests |
There was a problem hiding this comment.
BsonSerializationException class looks exactly like BsonInternalException. Should we make tests similar as well? Somehow BsonInternalExceptionTests has much more test cases.
| public class BsonInternalExceptionTests | ||
| { | ||
| [Fact] | ||
| public void constructor_should_initialize_empty_instance() |
There was a problem hiding this comment.
I suppose test method name should start with capital letter.
| token.Type.Should().Be(JsonTokenType.DateTime); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
It looks like previous test validates token.Type as well. Either remove the check there or remove this entire test.
| { | ||
| // We'll test a few representative BsonValue types | ||
|
|
||
| // Arrange - Int32 |
There was a problem hiding this comment.
Same as above (multiple arrange-act-assert section. Split into several tests?)
| var subject = new DictionaryInterfaceImplementerSerializer<Dictionary<string, int>>(dictionaryRepresentation); | ||
|
|
||
| // Assert | ||
| subject.Should().NotBeNull(); |
There was a problem hiding this comment.
Here and in other places: Does such check has any sense? Is there any way in C# to return null from the object constructor?
|
|
||
| var bson = obj.ToBson(); | ||
| var rehydrated = BsonSerializer.Deserialize<T>(bson); | ||
| Assert.IsType<Dictionary<object, object>>(rehydrated.D); |
There was a problem hiding this comment.
Should we use fluent assertions?
| var result = subject.Deserialize(context, args); | ||
|
|
||
| // Assert | ||
| result.Should().Be(expectedResult); |
There was a problem hiding this comment.
We should not compare results, we have to Verify if the method with proper arguments was called on the mock.
| var mockArraySerializer = new Mock<IBsonSerializer<string>>(); | ||
| mockArraySerializer.As<IBsonArraySerializer>() | ||
| .Setup(s => s.TryGetItemSerializationInfo(out It.Ref<BsonSerializationInfo>.IsAny)) | ||
| .Returns((out BsonSerializationInfo info) => |
There was a problem hiding this comment.
We do not really need any return value. Verifying if proper method was called should be enough.
No description provided.