-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Date functions #23
Added Date functions #23
Conversation
|
||
if (!typeof(DateTime).IsAssignableFrom(childexpression.Type) && !typeof(DateTimeOffset).IsAssignableFrom(childexpression.Type)) | ||
{ | ||
throw new Exception("Type does not support the 'day' function."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these perform this check? what about throwing an exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think this is valid... the exception handling in Linq to Querystring generally at the moment is not good so any improvements are welcomed! There are a few test files called 'Exceptions.cs' which verify these, so might be good to update those as well.
I added in a new exception type and unit tests to verify it gets thrown. I wasn't sure how detailed to get with those so I just ran them against a single string type. I also rebased to cleanup the commit history. |
I forgot to mention that locally I updated MSpec to the latest beta. This was the only way I could get resharper to run my tests since I have v8.1 installed. The MSTest runner was horrible and wouldn't run the tests right. If there's any inconsistencies between my tests and yours this is most likely why. If you'd like a PR for these changes I can submit another since I already made them. |
Cool, here goes nothing! |
Added Date functions
I seem to be having troubles with Mspec since the pull, it can't seem to find any of the assertion extension methods! Would this be related to the latest beta? |
The issue is probably due to how they're restructuring the MSpec project. I Had to add Machine.Specifications.Should to get the assertion methods back. In there they had If it helps here's the changes I made to update MSpec #24 |
Thanks! And congratulations for sending the first (and second) pull request for the project :) |
this is for review while I work through adding the rest of the functions in