Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Test on HttpRequestBodyConverter #148

Merged

Conversation

xpicio
Copy link
Contributor

@xpicio xpicio commented Aug 26, 2016

Hello, i just added more test on HttpRequestBodyConverter. About MultiPartFormHttpRequestBodyConverter i do not have any particular idea about test, both Nancy and ASP.NET split multipart/form-data in Form and Files collections so there is not any special test to do on it. If you have any idea about more test to add, let me know.


This change is Reviewable


namespace SharpRaven.UnitTests.Data
{
[TestFixture]
public class HttpRequestBodyConverterTests
{
#region TestCaseData
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did ReSharper add this region according to the DotSettings profile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is a my habit, usually i wrap private code on a dedicated region. Can i remove it if you want. If you are using a code convention let me know i will happy to follow it :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove it. I believe the associated ReSharper settings should place private classes at the bottom of the parent class, if not, let me know and I'll adjust.

@@ -60,6 +63,52 @@ public void Convert_Form_ReturnsForm()


[Test]
[TestCaseSource(typeof(JsonTestCase), "TestCasesContentType")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be easier to read if you just used [TestCase()] with the content type strings directly in the attribute, instead of extracting them to a source type? I'd understand it if the source data was more complex, but since it's just a string... 😃

Copy link
Contributor Author

@xpicio xpicio Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is 😄

@asbjornu
Copy link
Contributor

asbjornu commented Sep 5, 2016

@xpicio Excellent, thanks!

@asbjornu asbjornu merged commit 8a46f7b into getsentry:develop Sep 5, 2016
@xpicio xpicio deleted the feature/test_on_httprequestbodyconverter branch September 6, 2016 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants