[CSHARP-1029] Introduce interfaces for IBsonReader/IBsonWriter Created: 12/Aug/14  Updated: 02/Apr/15  Resolved: 03/Dec/14

Status: Closed
Project: C# Driver
Component/s: BSON
Affects Version/s: 1.9.2
Fix Version/s: 2.0

Type: Improvement Priority: Major - P3
Reporter: Svetoslav Milenov Assignee: Robert Stam
Resolution: Done Votes: 0
Labels: bson
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible

 Description   

To enable easier testing of serializers (along with other benefits), interfaces should be introduced where BsonReader and BsonWriter are currently being used.

Old

In order to unit test a custom bson serializers, a mock of BsonReader and BsonWriter need to be provided.

If the custom serializer depends on the CurrentBsonType or State properties, it can not be tested easily, as these properties are not virtual, and have protected setters.

There is an ugly workaround to create your own abstract inheritors of the above classes, where one can provide a public method to set these properties, and then for the test to mock that inheritors.

A better approach would be to make these properties virtual, providing for a better user code testability.



 Comments   
Comment by Githook User [ 03/Dec/14 ]

Author:

{u'username': u'rstam', u'name': u'rstam', u'email': u'robert@robertstam.org'}

Message: CSHARP-1029: Introduce IBsonReader and IBsonWriter interfaces.
Branch: master
https://github.com/mongodb/mongo-csharp-driver/commit/f5b902c65f52d92e380888521e2bedd69a0982f4

Comment by Craig Wilson [ 12/Aug/14 ]

We have discussed changing these to interfaces as well, just haven't pulled the trigger. Too many other, more important things to do.

While we can certainly debate the philosophy of what a pure unit test is, our current ones don't require any extrenal dependencies other than a working BsonWriter/BsonReader. And since those can be instantiated based on byte arrays or from parsing json fairly easily, we believe it much more clear to be testing with actual implementations of these classes. I do, however, understand your dilemna that all you want to do is test something small.

So... I don't think making these virtual is the right thing. The right thing is to use interfaces for BsonWriter/BsonReader. We are not at a point where we are able to discuss this change, so please don't go and do this. I am going to change this ticket's description to Introduce interfaces for BsonReader and BsonWriter to remind us this is important.

Comment by Svetoslav Milenov [ 12/Aug/14 ]

Hi Craig,
Actually, if you need only to check if the value to be deserialized is null (i.e. BsonType.Null) in order to perform something, there is no need to have the inner working, and mocking makes sense. At least for pure unit testing POV. Same for all other abstract/virtual methods, if the custom serializer use them.

I checked the tests you mention, but they are not "pure" unit tests.

Anyway, the change I proposed is the least impact on the code base. A proper solution would be to change the signature of IBsonSerializer methods to IBsonReader, and IBsonWriter, and make BsonWriter and BsonReader abstract classes to implement these interfaces. That way there will be a better decoupling. But this was a much bigger change, affecting too many files.

If you think such a change is acceptable. I can change my pull request and implement it.

Comment by Craig Wilson [ 12/Aug/14 ]

Hi Svetoslav,

Thanks for the PR and suggestion. Unfortunately, there is a lot of behavior in these base classes and mocking them to test your implementation wouldn't prove much. I'd suggest looking at how our internall implemented serializers are tested, and do that. Since they don't integrate with anything (mongodb, files, etc...), this isn't a problem provides a much more robust and provable implementation.

Craig

Comment by Svetoslav Milenov [ 12/Aug/14 ]

Pull request added: https://github.com/mongodb/mongo-csharp-driver/pull/186

Generated at Wed Feb 07 21:38:29 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.