Added support for checking a file content result's textual contents. #24
Conversation
|
You mentioned in issue #3 that we should output the encoding in the error message. I want to ask you - what should the message be? I am struggling to think of a way to make the error message make sense especially in cases like this: http://i.imgur.com/6sgBTvO.png |
|
Interesting re: that screenshot. I guess there is no way for us to know what encoding format was used so all we can do is output using the one they provided? So maybe that message would end up:
|
|
Also, pomodoro - nice! |
| throw new ActionResultAssertionException(string.Format("Expected file to be of content type '{0}', but instead was given '{1}'.", contentType, fileResult.ContentType)); | ||
| } | ||
|
|
||
| if (encoding == null) encoding = Encoding.UTF8; |
There was a problem hiding this comment.
I prefer single line ifs to be indented so it's easy to see it's a block when scanning down the file
|
I'm pretty happy with this; would you say it's good to merge? I need to set up PR auto-builds for this project :P |
|
Ignore that build error. It actually passed. |
Removed superflous using directives. Moved test method into appropriate region. Indented if statement (as per code review.) Removed unintended whitespace between region. Corrected test name. Corrected errant test body formatting.
I think this is a good idea and I want to implement it but before I do, I think it might be worth discussing this a little more. Ambiguity Encoding Name
What do you want to map
Could that be a bit confusing initially though? It is not too much of a leap from Of course none of this will be valid if we pursue a different error message format or.. Leave the error message as is? As I explore how to do this I get the impression that it is always going to be a trade off and think it might be better to not "interfere" as it were. I want to hear your thoughts though. I do not have the foresight to say really. If you want to roll with your proposed error message then I will implement it. Like I said, I like it a lot. Let me know! |
|
Really great points. My gut feel is telling me to start with the simplest thing and then if someone complains try and make it more complex. Also, anyone who isn't using UTF-8 is probably doing something crazy and they should probably understand what's going on if there are encoding issues. In terms of how to represent the encoding as a string I'd use the |
I am of the same opinion. Would you agree that the simplest thing is to not print the encoding? |
|
Probably :) |
Added support for checking a file content result's textual contents.
|
It is like you say - we can revisit this when needed. And here's to my first merge 🍻 |
|
:) Whenever you want the library deployed to nuget ping me and I can click the button on the ci server. Alternatively we can set up auto-deploy to nuget. Given this is a new feature it probably makes sense to bump the minor version number too (I'd like to follow semver from now on). The CI server uses GitVersion to version it. It auto-bumps patch for us, we can use NextVersion.txt to bump minor or major. |
No description provided.