Skip to content
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

Revert back to parameter logging format from v1 and update test baselines #8629

Merged
merged 1 commit into from
May 28, 2017

Conversation

ajcvickers
Copy link
Contributor

Issue #8456

We previously used a different format for parameter logging in our tests when compared to product code logging. Now we use the same format everywhere. This change updates the product format back to what it was, and updates the test baselines to reflect this.

…ines

Issue #8456

We previously used a different format for parameter logging in our tests when compared to product code logging. Now we use the same format everywhere. This change updates the product format back to what it was, and updates the test baselines to reflect this.
Copy link
Contributor

@divega divega left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -989,7 +989,7 @@ public static TheoryData CommandActions
foreach (var item in log.Skip(1))
{
Assert.EndsWith(
@"[Parameters=[FirstParameter: 17], CommandType='0', CommandTimeout='30']
@"[Parameters=[FirstParameter='17'], CommandType='0', CommandTimeout='30']
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Did we ever consider not including CommandType in this case? I believe it will be 0 in not specified, although documentation says default is Table, neither is very interesting. Also, do we always log the numeric value or do we log the name of the enum member?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@divega Don't know. We never set it, so it will always be zero, which is not a valid enum value and hence is shown like this. This was done a long time ago--I doubt it was really thought about, but I don't know.

@ajcvickers ajcvickers merged commit 4ea6a93 into dev May 28, 2017
@smitpatel smitpatel deleted the DontQuoteMeOhGoOnThen0527 branch June 2, 2017 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants