-
Notifications
You must be signed in to change notification settings - Fork 191
Implement OwinEnvironment
IEnumerable.GetEnumerator()
#789
Conversation
@kchanlee, |
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.
Looking good. Minor comments
[Fact] | ||
public void OwinEnvironmentImpelmentsGetEnumerator() | ||
{ | ||
HttpContext context = CreateContext(); |
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.
style: use var
whenever possible instead of the explicit type. I know that the rest of the test class doesn't follow this guideline but it's never too late to start.
Also, combine: var owinEnvironment = new OwinEnvironment(CreateContext());
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.
Updated
IEnumerator<KeyValuePair<string, object>> e1 = owinEnvironment.GetEnumerator(); | ||
Assert.NotNull(e1); | ||
|
||
foreach (var kv in owinEnvironment) { } // This should not throw 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.
These checks are not really useful. It's enough to check that the enumerators are not null.
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.
Updated
HttpContext context = CreateContext(); | ||
OwinEnvironment owinEnvironment = new OwinEnvironment(context); | ||
|
||
IEnumerator<KeyValuePair<string, object>> e1 = owinEnvironment.GetEnumerator(); |
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.
Combine: Assert.NotNull(owinEnvironment.GetEnumerator());
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.
Updated
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.
Thanks for the contribution, we'll merge soon.
Implement
OwinEnvironment
IEnumerable.GetEnumerator()
Related to Issue 788